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

[Philippe Mathieu-Daudé]

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

> 
> That results in:
> $ ./xscv_non_signaling
> xscvspdpn: 0x7ff00000400000000000000000000000
> xscvdpspn: 0x7f8000007f8000000000000000000000
> $ qemu-ppc64le ./xscv_non_signaling
> xscvspdpn: 0x7ff80000400000000000000000000000
> xscvdpspn: 0x7f8000007f8000000000000000000000
> 
> PowerISA is more descriptive of xscvdpspn wrt SNaN but doesn't say
> anything about NaNs in xscvspdpn description. Should we match the
> hardware behavior? If so, does it worth adding a new flag in
> float_status for a single instruction? We could also handle that in
> helper_xscvdpspn, e.g.:
> 
> float32 sp = xb >> 32;
> float64 dp;
> 
> dp = float32_to_float64(xb >> 32, &tstat);
> if (float32_classify(sp) == is_snan) {
>     dp &= ~(1ULL << 51);
>     dp |= (dp & 0x7ffffffffffffULL) == 0;
> }
> 
> return dp;
> 
> but it feels kind hacky.