Jiufu Guo <guoji...@linux.ibm.com> writes: Backported to GCC 9, preapproved by Segher.
Thanks, Jiufu > 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
