Re: [Qemu-devel] [PATCH v4 32/49] tests/tcg/arm: add fcvt test cases for AArch32/64

[Alex Bennée]

Philippe Mathieu-Daudé <address@hidden> writes:

> On 05/17/2018 05:30 PM, Richard Henderson wrote:
>> On 05/17/2018 10:47 AM, Alex Bennée wrote:
>>> This runs through the usual float to float conversions and crucially
>>> also runs with ARM Alternative Half Precision Format.
>>>
>>> Signed-off-by: Alex Bennée <address@hidden>
>>> [rth: tweak vcvtb.f16.f32/vctb.f32.f16 code and regen]
>>> Signed-off-by: Richard Henderson <address@hidden>
>>>
>>> ---
>>> v4
>>>   - add fcvt.ref and check results against it
>>>   - fix single_to_half, single_to_double conversions
>>>   - properly toggle AHP mode (fpsr->fpcr)
>>>   - more values around the AHP margins
>>>   - add INF/NAN/SNAN inputs
>>>   - build for ARM and AArch64
>>>   - fix bug for hex literals
>>>   - add float-to-int
>>>   - checkpatch fix
>>> ---
>>
>> Reviewed-by: Richard Henderson <address@hidden>
>
> Thanks Richard, I was not very motivated to review each line of the
> fcvt.ref files =)
>
>> diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target
>> index 9d2b551732..7bb777f442 100644
>> --- a/tests/tcg/arm/Makefile.target
>> +++ b/tests/tcg/arm/Makefile.target
>> @@ -8,7 +8,9 @@ ARM_SRC=$(SRC_PATH)/tests/tcg/arm
>>  # Set search path for all sources
>>  VPATH               += $(ARM_SRC)
>>
>> -TESTS += hello-arm test-arm-iwmmxt
>> +ARM_TESTS=hello-arm test-arm-iwmmxt
>> +
>> +TESTS += $(ARM_TESTS) fcvt
>>
>>  hello-arm: CFLAGS+=-marm -ffreestanding
>>  hello-arm: LDFLAGS+=-nostdlib
>> @@ -24,3 +26,14 @@ run-test-mmap: test-mmap
>>      $(call quiet-command, $(QEMU) -p 8192 $< 8192 > test-mmap-8192.out,
> "TEST", "$< (8k pages) on $(TARGET_NAME)")
>>      $(call quiet-command, $(QEMU) -p 16384 $< 16384 >
> test-mmap-16384.out, "TEST", "$< (16k pages) on $(TARGET_NAME)")
>>  endif
>> +
>> +ifeq ($(TARGET_NAME), arm)
>> +fcvt: LDFLAGS+=-lm
>> +# fcvt: CFLAGS+=-march=armv8.2-a+fp16 -mfpu=neon-fp-armv8
>
> Alex, What is your idea here, enable this later?
> Maybe add some TODO comment around..

How about:

# AArch32 doesn't support all combinations of FP16<->FOO in it's base
# profile. Once the AArch32 profile is caught up we should build an
# additional binary and reference to test the additional combinations.

I backed the change out when I realised the stretch compiler didn't
support the same range as my local (18.04) gcc's.

>
> Except this nit, for both Makefile.target:
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>
>> +
>> +run-fcvt: fcvt
>> +    $(call quiet-command, \
>> +            $(QEMU) $< > fcvt.out && \
>> +            diff -u $(ARM_SRC)/fcvt.ref fcvt.out, \
>> +            "TEST", "$< (default) on $(TARGET_NAME)")
>> +endif
>
> $ make -j1 run-tcg-tests-aarch64-linux-user
>   BUILD   debian9
>   BUILD   debian-arm64-cross
>   CROSS-BUILD aarch64 guest-tests with docker qemu:debian-arm64-cross
>   BUILD   debian9
>   BUILD   debian-arm64-cross
>   CROSS-BUILD aarch64 guest-tests with docker qemu:debian-arm64-cross
>   RUN-TESTS for aarch64
>   TEST    test-mmap (default) on aarch64
>   TEST    test-mmap (8k pages) on aarch64
>   TEST    test-mmap (16k pages) on aarch64
>   TEST    test-mmap (32k pages) on aarch64
>   TEST    sha1 on aarch64
>   TEST    linux-test on aarch64
>   TEST    testthread on aarch64
>   TEST    fcvt (default) on aarch64
>   TEST    sysregs on aarch64
>
> \o/
>
> Tested-by: Philippe Mathieu-Daudé <address@hidden>


--
Alex Bennée