Re: Adding binutils to the GNU Toolchain buildbot on sourceware

Jan Beulich via Binutils <[email protected]>
Newsgroups gmane.comp.gnu.binutils,gmane.comp.gdb.devel
Message-ID <[email protected]>
On 26.04.2022 17:47, H.J. Lu wrote:
> On Tue, Apr 26, 2022 at 6:49 AM Jan Beulich <[email protected]> wrote:
>>
>> On 26.04.2022 14:27, Nick Clifton wrote:
>>>> For gas/testsuite/gas/i386/rept I did already suggest [1] to simply purge
>>>> the test as unreliable. I also put under question the purpose that it was
>>>> originally added for.
>>>>
>>>> [1] https://sourceware.org/pipermail/binutils/2022-April/120339.html
>>>
>>> Hmm, I seem to have missed this one.  In fact the entire patch series.  Sorry.
>>> The series looks good to me, so please go ahead and apply it in its entirety
>>
>> No problem. With my new powers I had committed it already.
>>
>>> As for disabling the rept because it is so memory expensive: I think that we
>>> used to.have an environment variable called something like RUN_EXPENSIVE_TESTS
>>> that had to be set before certain tests were run.  Checking the sources though
>>> I cannot find it, so maybe I am imagining things.  I still might be a good idea
>>> though.  If we use it consistently in the binutils testsuites for the big tests
>>> then users will probably appreciate the facility.
>>
>> But a test which is run by almost nobody is more likely to break. Also I
>> think "expensive" has multiple dimensions (memory and time at least), and
>> depending on the system one may want to run (or suppress) one but not the
>> other kind. For the test in question, it is both memory and cycles hungry,
>> so there the distinction may not matter.
>>
>> But I'd like to raise the question again: Is what the test was added for
>> actually a useful thing to test, at the risk of the test failing simply
>> because there's too little memory available? Iirc the problem was non-
>> graceful error handling. But the test does not check that the error in
>> question now is handled gracefully; it expects that there be no error.
>>
>>
> 
> gas/testsuite/gas/i386/rept.s failed to assemble before the fix:
> 
> commit 39a45edc073226e748566b497c216830cec119c4
> Author: Alan Modra <[email protected]>
> Date:   Thu Jun 7 12:47:23 2012 +0000
> 
>             PR gas/14201

The problem wasn't that the file failed to assemble, but that it triggered
an assertion. With a suitably large repeat count, such a construct is
guaranteed to fail to build. And it's probably not just coincidence that
in the test you did use a repeat count only little more than half of what
the bug report had. Hence what the test should have checked for (and what
reporter as well as Alan also said) is that a proper error would result
instead of an assertion.

> We can limit it to 64-bit only if it helps.

I don't think that would make it any better.

Jan
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.