On 12/13/20 2:56 PM, Joel Brobecker wrote:
> Hi Tom,
> 
>> When running test-case gdb.base/endianity.exp using gcc-4.8, we get:
>> ...
>> (gdb) x/x &o.v^M
>> 0x7fffffffd120: 0x00000004^M
>> (gdb) XFAIL: gdb.base/endianity.exp: x/x &o.v
>> x/xh &o.w^M
>> 0x7fffffffd124: 0x0003^M
>> (gdb) FAIL: gdb.base/endianity.exp: x/xh &o.w
>> ...
>>
>> The gcc 4.8 compiler does not support the scalar_storage_order attribute, so
>> the testcase is compiled without that attribute, and the expected results are
>> different.
>>
>> This is why there's the first XFAIL, and we could xfail the second FAIL for the
>> same reason.
>>
>> Instead, fix this by adapting the expected values based on whether the attribute
>> has been used in endianity.c.
>>
>> Also, remove hard-coding of the byte order in the expected memory printing.
>>
>> Tested on x86_64-linux, with gcc-4.8, gcc-7, and clang-10.
>>
>>
>> Any comments?
> 
> For me, the whole point of this testcase is to test SSO, so if
> the compiler doesn't support it, the testcase loses its value
> entirely (to my eyes anyway). As a result of this, I dont' think
> bringing the extra complexity that you are suggesting is bringing
> any value -- I might argue that it's now hard to read the testcase
> an understand what we're trying to do (sorry!).
> 

Np, that's also good feedback, thanks.

> In my opinion, rather than an XFAIL, we should just only do
> the second half of the testcase if the compiler supports it,
> than xfailing the tests. So I would do:
> 
> if { ([test_compiler_info {gcc-[0-5]-*}] || ![test_compiler_info gcc*]) } {
>     # The rest of the testcase requires Scalar Storage Order support.
>     # This compiler does not support it, so skip the rest.
>     return
> }
> 

Ack, committed as below.

Thanks,
- Tom