Re: [RFC PATCH] target/ppc: do not silence snan in xscvspdpn

[Alex Bennée]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> On 12/13/21 21:15, Matheus K. Ferst wrote:
>> On 13/12/2021 09:36, Philippe Mathieu-Daudé wrote:
>>> On 12/13/21 13:13, matheus.ferst@eldorado.org.br wrote:
>>>> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>>>>
>>>> The non-signalling versions of VSX scalar convert to shorter/longer
>>>> precision insns doesn't silence SNaNs in the hardware. We are currently
>>>> honoring this behavior in xscvdpspn, since helper_xscvdpspn handles the
>>>> conversion with extracts/deposits/etc. OTOH, xscvspdpn uses
>>>> float32_to_float64 that calls parts_float_to_float, which uses
>>>> parts_return_nan that finally calls parts_silence_nan if the input is an
>>>> SNaN.
>>>>
>>>> To address this problem, this patch adds a new float_status flag
>>>> (return_snan) to avoid the call to parts_silence_nan in the
>>>> float_class_snan case of parts_return_nan.
>>>>
>>>> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
>>>> ---
>>>> This behavior was observed in a Power9 and can be reproduced with the
>>>> following code:
>>>>
>>>> int main(void)
>>>> {
>>>>      __uint128_t t, b = 0x7f80000200000000;
>>>>
>>>>      asm("xscvspdpn %x0, %x1\n\t"
>>>>          : "=wa" (t)
>>>>          : "wa" (b << 64));
>>>>      printf("0x%016" PRIx64 "%016" PRIx64 "\n",
>>>>             (uint64_t)(t >> 64), (uint64_t)t);
>>>>
>>>>      b = 0x7ff0000000000002;
>>>>      asm("xscvdpspn %x0, %x1\n\t"
>>>>          : "=wa" (t)
>>>>          : "wa" (b << 64));
>>>>      printf("0x%016" PRIx64 "%016" PRIx64 "\n",
>>>>             (uint64_t)(t >> 64), (uint64_t)t);
>>>>
>>>>      return 0;
>>>> }
>>>
>>> Why not add this test in tests/tcg/ppc64le/ ?
>> 
>> I'll add it in v2. Is it ok to use __uint128_t in tests?
>
> What about:
>
>   int main(void)
>   {
>   #ifndef __SIZEOF_INT128__
>       printf("uint128_t not available, skipping...\n");
>   #else
>       ...
>   #endif
>       return 0;
>   }

We have a tests/tcg/configure.sh which does feature tests although it is
mainly testing for the presence of compiler target flags.  We do this
because while the docker compilers are all pretty modern the user might
be using their own cross compiler.

I'm generally not keen on the tests silently skipping because it gives a
false impression things have been tested. If it is possible to avoid
INT128 shenanigans to load the values into the inline assembler lets do
that, otherwise lets feature test and ifdef a skip-test in the makefile.

-- 
Alex Bennée