On 2/7/25 11:48 PM, Michael Meissner wrote: > On Fri, Feb 07, 2025 at 05:51:19PM -0600, Peter Bergner wrote: >> On 2/7/25 4:02 PM, Michael Meissner wrote: >>> (define_predicate "invert_fpmask_comparison_operator" >>> - (match_code "ne,unlt,unle")) >>> + (ior (match_code "ne") >>> + (and (match_code "unlt,unle") >>> + (match_test "!HONOR_NANS (DFmode) || !TARGET_P9_VECTOR")))) >> >> Is it always safe to use DFmode here in the HONOR_NANS macro? >> Meaning does it always give the same answer as using SFmode, TFmode, >> IFmore and KFmode? Ditto for the other use of HONOR_NANS (DFmode). > > The problem is we do not have the original floating point mode. The mode in > question is CCFPmode, i.e. the mode used for setting the CR registers. The > code originally just used !flag_finite_math. I could go back to that, I > thought HONOR_NANS was clearer.
I understand we don't have the floating point mode and I'm all for clearer code. I just trying to confirm that HONOR_NANS (DFmode) == HONOR_NANS (SFmode), etc. I'm assuming that's true, but wanted to be sure. > I can do that, but I would problably named it something like > ordered_compare_ok. Just ordered by itself would say to me that only ordered > comparisons are allowed, when the majority of comparisons that the function > sees are the normal unordered comparisons (i.e. EQ, NE, GT, GE, LE, and LT). ordered_compare_ok sounds good to me. >>> + However, this is not safe for ordered comparisons (i.e. for isgreater, >>> + etc.) starting with the power9 because ifcvt.cc will want to create >>> a fp >>> + cmove, and the x{s,v}cmp{eq,gt,ge}{dp,qp} instructions will trap if >>> one of >>> + the arguments is a signalling NaN. */ >> >> I think this could use a little work smithing and there is some stray >> whitespace. You also explicitly mention signalling NaN, but the code >> uses HONOR_NANS, not HONOR_SNANS. Is that intentional? > > As I said in my previous reply, it is intentional. The option > -fsignaling-nans > is not normally set. However, I felt that if the user explicitly used one of > the ordered comparisons (i.e. isgreater in this case) that the compiler should > explicitly generate code that is safe for using signalling NaNs because that > is > how those functions are describe. So use of HONOR_NANS was intentional. Then why mention "signalling NaN" in the comment? Should that just be s/signalling NaN/NaN/ then? I still think a little wordsmithing would be nice, so the paragraph flowed better. Let me know if you'd rather have me give it a go. Peter