Re: [Xen-devel] [PATCH] tests/cpu-policy: Skip building on older versions of GCC

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/05/2019 15:19, Jan Beulich wrote:
>>>> On 24.05.19 at 15:29, <andrew.cooper3@xxxxxxxxxx> wrote:
>> GCC 4.4 (as included in CentOS 6) is too old to handle designated 
>> initialisers
>> in anonymous unions.  As this is just a developer tool, skip the test in this
>> case, rather than sacraficing the legibility/expresibility of the test cases.
>>
>> This fixes the Gitlab CI tests.
>>
>> While adding this logic to cpu-polcy, adjust the equivelent logic from
>> x86_emulator on which this was based.  Printing:
>>
>>   Test harness not built, use newer compiler than "gcc"
>>
>> isn't helpful for anyone unexpectedly encountering the error.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Fundamentally
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> But there are remarks:
>
>> --- a/tools/tests/cpu-policy/Makefile
>> +++ b/tools/tests/cpu-policy/Makefile
>> @@ -1,8 +1,20 @@
>>  XEN_ROOT = $(CURDIR)/../../..
>>  include $(XEN_ROOT)/tools/Rules.mk
>>  
>> +TARGET-y := test-cpu-policy
>> +
>> +# For brevity, these tests make extensive use of designated initialisers, 
>> but
>> +# GCCs older than 4.6 can't cope.  Ignore the test in this case.
> Designated initializers alone are no problem for old gcc. The issue is
> with ones used for sub-structures/-unions without field name.
> Perhaps worth slightly extending the comment to this effect?

" in anonymous unions" ?  I can never remember exactly which bit it
chokes on, but I think there are two different ones in practice which
interfere.

>
>> --- a/tools/tests/x86_emulator/Makefile
>> +++ b/tools/tests/x86_emulator/Makefile
>> @@ -97,7 +97,7 @@ $(foreach flavor,$(SIMD) $(FMA),$(eval $(call 
>> simd-check-cc,$(flavor))))
>>  TARGET-$(shell echo 'asm("{evex} vzeroall");' | $(CC) -x c -c -o /dev/null 
>> - || echo y) :=
>>  
>>  ifeq ($(TARGET-y),)
>> -$(warning Test harness not built, use newer compiler than "$(CC)")
>> +$(warning Test harness not built, use newer compiler than $(CC) $(shell 
>> $(CC) -dumpversion) and an "{evex}" capable assembler)
>>  endif
> I appreciate the idea of providing mode information, but I'm afraid
> this is going to be clumsy in the other direction now:
>
> "Test harness not built, use newer compiler than gcc-4.8 4.8 and ..."
>
> Naming the compiler binary, I thought, allows the user to figure
> out the version easily enough. Therefore, please consider
> dropping that part again.

I'm afraid you have a selection bias here.  Your compiler binaries may
have a version suffix, but the overwhelming majority of people who are
going to hit that error and need to figure out what to do will be using
their system-provided binaries, as per the commit message.

What about:

  ... than "$(CC)" (version $(shell $(CC) -dumpversion)) and ...

which should (in your example) render as:

  ... than "gcc-4.8" (version 4.8) and ...

?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel