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
