Segher Boessenkool <seg...@kernel.crashing.org> writes:
> Hi!
>
> On Thu, Mar 05, 2020 at 10:46:58AM +0800, Jiufu Guo wrote:
>> PR93709 mentioned regressions on maxlocval_4.f90 and minlocval_f.f90 which
>> relates to max of '-inf' and 'nan'. This regression occur on P9 which has
>> new instruction 'xsmaxcdp/xsmincdp'.
>> The similar issue also could be find on `a < b ? b : a` which is also
>> generated as `xsmaxcdp` under -O2 for P9. This instruction `xsmaxcdp`
>> more like C/C++ semantic (a>b?a:b). A testcase is added for this issue.
>>
>> The following patch improve code to check -+0 and NaN before 'smax/smin' to
>> be generated for those cases.
>
>> - else if (rtx_equal_p (op1, true_cond) && rtx_equal_p (op0, false_cond))
>> + /* Only when -fno-signed-zeros and -ffinite_math_only are in effect,
>> + `op0 < op1 ? op1 : op0` works like `op1 > op0 ? op1 : op0` which
>> + could use smax;
>> + `op0 > op1 ? op1 : op0` works like `op1 < op0 ? op1 : op0` which
>> + could use smin. */
>> + else if (rtx_equal_p (op1, true_cond) && rtx_equal_p (op0, false_cond)
>> + && (flag_finite_math_only && !flag_signed_zeros))
>> max_p = !max_p;
>
> I know I asked for it, but should this use HONOR_NANS (compare_mode)
> instead? Infinities will work fine? Just NaNs and zeros won't.
HONOR_NANS(mode) is `MODE_HAS_NANS (mode) && !flag_finite_math_only`.
We know the mode is SF or DF. Both maybe ok for current code.
I agree with you HONOR_NANS would be better, it is more generic for
front-end, gimple and rtl. And rs6000_emit_p9_fp_minmax maybe called
without checking mode in future code, in this case HONOR_NANS is
better.
I updated the code as:
```
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index f34e1ba70c6..b057f689b56 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -14836,7 +14836,11 @@ rs6000_emit_p9_fp_minmax (rtx dest, rtx op, rtx
true_cond, rtx false_cond)
if (rtx_equal_p (op0, true_cond) && rtx_equal_p (op1, false_cond))
;
- else if (rtx_equal_p (op1, true_cond) && rtx_equal_p (op0,
false_cond))
+ /* Only when NaNs and signed-zeros are not in effect, smax could be
+ used for `op0 < op1 ? op1 : op0`, and smin could be used for
+ `op0 > op1 ? op1 : op0`. */
+ else if (rtx_equal_p (op1, true_cond) && rtx_equal_p (op0, false_cond)
+ && !HONOR_NANS (compare_mode) &&
!HONOR_SIGNED_ZEROS(compare_mode))
max_p = !max_p;
else
```
This code works fine. I'm going to submit it.
Thanks!
Jiufu Guo
>
> Okay for trunk with that change (if it works :-) ) Thanks!
>
>
> Segher
