Re: [PATCH v2 6/6] tests/tcg/arm: Tests for new FPRCVT instructions

["Alex Bennée" <alex.bennee@linaro.org>]

Jim MacArthur <jim.macarthur@linaro.org> writes:

> We autodetect the presence of FPRCVT in the test cross compiler,
> which is a recent feature in GCC and not supported by many distros
> yet. If this is in place, we compile the existing fcvt.c test with
> an extra compiler flag which uses the new SIMD instructions; the
> output from the test is unchanged.
>
> The existing [US]CVTF instructions do not have a test, so no new
> tests are added for the SIMD versions. They have been tested manually
> to check the new SIMD versions produce the same numerical results as
> the existing versions.
>
> Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
> ---
>  tests/tcg/aarch64/Makefile.target | 14 +++++++++++++-
>  tests/tcg/arm/fcvt.c              |  7 +++++++
>  2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
> index 6203ac9b51..32f2689273 100644
> --- a/tests/tcg/aarch64/Makefile.target
> +++ b/tests/tcg/aarch64/Makefile.target
> @@ -28,9 +28,21 @@ config-cc.mak: Makefile
>  	    $(call cc-option,-march=armv8.5-a,              CROSS_CC_HAS_ARMV8_5); \
>  	    $(call cc-option,-mbranch-protection=standard,  CROSS_CC_HAS_ARMV8_BTI); \
>  	    $(call cc-option,-march=armv8.5-a+memtag,       CROSS_CC_HAS_ARMV8_MTE); \
> -	    $(call cc-option,-Wa$(COMMA)-march=armv9-a+sme $$fnia, CROSS_AS_HAS_ARMV9_SME)) 3> config-cc.mak
> +	    $(call cc-option,-Wa$(COMMA)-march=armv9-a+sme $$fnia, CROSS_AS_HAS_ARMV9_SME); \
> +	    $(call cc-option,-march=armv9-a+fprcvt,         CROSS_CC_HAS_ARMV9_FPRCVT)) 3> config-cc.mak
>  -include config-cc.mak
>  
> +ifneq ($(CROSS_CC_HAS_ARMV9_FPRCVT),)
> +AARCH64_TESTS += fcvt-fprcvt
> +fcvt-fprcvt: LDFLAGS += -lm
> +fcvt-fprcvt: CFLAGS += $(CROSS_CC_HAS_ARMV9_FPRCVT) -DFPRCVT

Do you actually need $(CROSS_CC_HAS_ARMV9_FPRCVT) in the compile?
-DFPRCVT handles the compilation.

> +fcvt-fprcvt: fcvt.c
> +	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
> +run-fcvt-fprcvt: fcvt-fprcvt
> +	$(call run-test,$<,$(QEMU) $<)
> +	$(call diff-out,$<,$(AARCH64_SRC)/fcvt.ref)
> +endif
> +
>  ifneq ($(CROSS_CC_HAS_ARMV8_2),)
>  AARCH64_TESTS += dcpop
>  dcpop: CFLAGS += $(CROSS_CC_HAS_ARMV8_2)
> diff --git a/tests/tcg/arm/fcvt.c b/tests/tcg/arm/fcvt.c
> index ecebbb0247..7c0cc4367e 100644
> --- a/tests/tcg/arm/fcvt.c
> +++ b/tests/tcg/arm/fcvt.c
> @@ -171,8 +171,14 @@ static void convert_single_to_integer(void)
>  #if defined(__arm__)
>          /* asm("vcvt.s32.f32 %s0, %s1" : "=t" (output) : "t" (input)); */
>          output = input;
> +#else
> +#ifdef FPRCVT
> +        asm("fcvtzs d0, %s1\r\n"
> +            "fmov %0, d0" :
> +            "=r" (output) : "w" (input));
>  #else
>          asm("fcvtzs %0, %s1" : "=r" (output) : "w" (input));
> +#endif
>  #endif
>          print_int64(i, output);
>      }
> @@ -425,6 +431,7 @@ int main(int argc, char *argv[argc])
>      convert_double_to_integer();
>      convert_half_to_integer();
>  
> +

stray space

Otherwise looks good with the clobber fix.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


>      /* And now with ARM alternative FP16 */
>  #if defined(__arm__)
>      asm("vmrs r1, fpscr\n\t"

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro