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


Reply via email to