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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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?

> --- 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 unconditionally fine with the {evex} addition.

Jan



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