llvm dev - Aug 2021 - [cfe-dev] Intrinsic llvm::isnan

Regardless of everything, i would like to see a patch that restores
the -ffast-math handling, and *then* the RFC on what the is-nan check
should do when -ffast-math is present.
It is more than possible that the RFC will succeed,
but i don't think a change like that should happen the way it did.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949 was mentioned as
motivational, but i don't see any resolution there,
it's not even in "confirmed" state.

The change landed *after* the branch, so i think we are not too
constrained on the timeline for it. If there is no RFC before Aug 30'th,
i will be posting a patch to revert said change myself.


Roman.

On Tue, Aug 24, 2021 at 8:34 PM Serge Pavlov via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
>
> On Tue, Aug 24, 2021 at 10:59 PM Sanjay Patel via cfe-dev <cfe-dev at
lists.llvm.org> wrote:
>>
>> Thanks for making this clearer. So there's general consensus that
we need an intrinsic to represent isnan()...
>> 1. How many others like this do we need? (see the list that James
provided a couple of mails earlier)
>
>
> We need all classification functions: isnan, isinf, isfinite, isnormal,
issubnormal, iszero, fpclassify. In strict exception mode even iszero requires
special treatment, it cannot be implemented as compare with zero, because the
argument may be a signaling NaN.
>
>> 2. How do we introduce these and limit regressions? D104854 had clang
produce isnan() for all modes simultaneously, but that leads to problems like:
https://llvm.org/PR51556 . I suggest introducing these in LLVM only or in clang
with strictFP modes only as a 1st step, so we're not causing perf
regressions that require reverting large patches.
>
>
> The patch is already 3 weeks in the tree. PR51556 is the only problem so
far and it is a missed optimization, not malfunction. Besides, as it is said in
the ticket, "the SLP call vectorization fails as we don't properly
account for calls that have different return/arg types", so the problem
already existed, this intrinsic only elucidated it. I think if in a couple of
weeks no new problems will be found, work in this direction can be continued, as
if there were something broken, it would be already reported.
>>
>>
>> On Tue, Aug 24, 2021 at 8:26 AM Kevin Neal <Kevin.Neal at
sas.com> wrote:
>>>
>>> It probably doesn’t help that IEEE 754-2019 uses the word “signal”
to mean, paraphrased, “set a bit in a status register and continue producing a
result with no trap”, and that’s in the default FP environment. So IEEE
“signaling” is not Unix “signaling”.
>>>
>>>
>>>
>>> Once one picks up on that distinction the IEEE 754 document reads a
lot like Andy’s description of x86-64 behavior.
>>>
>>>
>>>
>>> From: Kaylor, Andrew <andrew.kaylor at intel.com>
>>> Sent: Monday, August 23, 2021 6:31 PM
>>> To: Arthur O'Dwyer <arthur.j.odwyer at gmail.com>; Sanjay
Patel <spatel at rotateright.com>
>>> Cc: Kevin Neal <Kevin.Neal at sas.com>; Wang, Pengfei
<pengfei.wang at intel.com>; llvm-dev at lists.llvm.org; Clang Dev
<cfe-dev at lists.llvm.org>
>>> Subject: RE: [cfe-dev] [llvm-dev] Intrinsic llvm::isnan
>>>
>>>
>>>
>>> EXTERNAL
>>>
>>> I think the confusion here has to do with what it means for the
optimizer to assume that “floating-point exceptions will be masked”. What I’m
about to say may be specific to the x86-64-based processor behavior because
that’s the one I know, but I think other architectures will have similar
behavior but possibly with different terms?
>>>
>>>
>>>
>>> If a floating point exception occurs in an operation and that
exception is masked in the floating point control registers, the status bit for
that exception will be set and nothing else will happen. If a floating point
exception occurs in an operation and that exception is NOT masked in the
floating point control registers, an exception handler will be called.
>>>
>>>
>>>
>>> So, what the current documentation means to say is that the
optimizer may assume that the status flags will not be read and that no
exception handler will be called if a floating point exception occurs.
Basically, it means that floating point exceptions will be ignored in all ways.
>>>
>>>
>>>
>>>
>>>
>>> > It's still not clear to me if there's a benefit from
having an intrinsic vs. one more exception mode ("none" or
"off").
>>>
>>>
>>>
>>> The key point to recognize here is that the metadata arguments to
the constrained intrinsics are intended to describe assumptions that can or
cannot be made. They are not intended to bring about the state that they
describe. So, for example, if the rounding mode argument is set to
“fpround.tonearest” that means the optimizer can assume that the processor is
set to use the “tonearest” rounding mode, but it does not enforce this. If we
see this flag during ISel, we can select an instruction that takes an explicit
rounding mode argument and use the “tonearest” value for that argument, but we
are also allowed to select an instruction that uses the rounding mode from the
MXCSR register and assume that when this instruction is executed that rounding
mode will be “tonearest”.
>>>
>>>
>>>
>>> If you need an intrinsic that is guaranteed not to raise
exceptions, that should be a distinct intrinsic from any similar intrinsic that
may raise exceptions. See, for example, llvm.experimental.constrained.nearbyint
and llvm.experimental.constrained.rint which differ only in exception behavior.
>>>
>>>
>>>
>>>
>>>
>>> -Andy
>>>
>>>
>>>
>>>
>>>
>>> From: Arthur O'Dwyer <arthur.j.odwyer at gmail.com>
>>> Sent: Monday, August 23, 2021 2:37 PM
>>> To: Sanjay Patel <spatel at rotateright.com>
>>> Cc: Kaylor, Andrew <andrew.kaylor at intel.com>; Kevin Neal
<Kevin.Neal at sas.com>; Wang, Pengfei <pengfei.wang at intel.com>;
llvm-dev at lists.llvm.org; Clang Dev <cfe-dev at lists.llvm.org>
>>> Subject: Re: [cfe-dev] [llvm-dev] Intrinsic llvm::isnan
>>>
>>>
>>>
>>> On Mon, Aug 23, 2021 at 3:42 PM Sanjay Patel via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
>>>
>>> Ok, does this edit to the LangRef make sense for the definition of
"ignore":
>>>
>>> "optimization passes may assume that the exception status
flags will not be read and that floating-point exceptions **will** be
masked" -->
>>>
>>> "optimization passes may assume that the exception status
flags will not be read and that floating-point exceptions **may** be
masked"
>>>
>>>
>>>
>>> I haven't been following the technical details, but in terms of
the English documentation, it makes no sense to say that someone "may
assume that [X] may happen." Either [X] always happens, in which case
optimization passes may safely assume that it happens; or [X] never happens, in
which case optimization passes may safely assume that it does not happen; or
else [X] sometimes happens and sometimes doesn't, in which case
optimizations passes must not assume anything about [X].
>>>
>>>
>>>
>>> So you might say: "optimization passes may assume that the
exception status flags will not be read. Floating-point exceptions might or
might not be masked, depending on [____]" (and then mention the relevant
variable, such as "instruction set" or "optimization level"
or whatever).
>>>
>>>
>>>
>>> HTH,
>>>
>>> Arthur
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

There is no problem to restore the previous behavior if -ffast-math is
specified. But why cannot this RFC be used  for the discussion of this
behavior?

Making a real check instead of optimizing out the function seems safe, as
it does not break functionality. Could you please share, what is your
concern?

Thanks,
--Serge


On Wed, Aug 25, 2021 at 12:53 AM Roman Lebedev <lebedev.ri at gmail.com>
wrote:
> Regardless of everything, i would like to see a patch that restores
> the -ffast-math handling, and *then* the RFC on what the is-nan check
> should do when -ffast-math is present.
> It is more than possible that the RFC will succeed,
> but i don't think a change like that should happen the way it did.
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949 was mentioned as
> motivational, but i don't see any resolution there,
> it's not even in "confirmed" state.
>
> The change landed *after* the branch, so i think we are not too
> constrained on the timeline for it. If there is no RFC before Aug
30'th,
> i will be posting a patch to revert said change myself.
>
>
> Roman.
>
> On Tue, Aug 24, 2021 at 8:34 PM Serge Pavlov via cfe-dev
> <cfe-dev at lists.llvm.org> wrote:
> >
> > On Tue, Aug 24, 2021 at 10:59 PM Sanjay Patel via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
> >>
> >> Thanks for making this clearer. So there's general consensus
that we
> need an intrinsic to represent isnan()...
> >> 1. How many others like this do we need? (see the list that James
> provided a couple of mails earlier)
> >
> >
> > We need all classification functions: isnan, isinf, isfinite,
isnormal,
> issubnormal, iszero, fpclassify. In strict exception mode even iszero
> requires special treatment, it cannot be implemented as compare with zero,
> because the argument may be a signaling NaN.
> >
> >> 2. How do we introduce these and limit regressions? D104854 had
clang
> produce isnan() for all modes simultaneously, but that leads to problems
> like: https://llvm.org/PR51556 . I suggest introducing these in LLVM only
> or in clang with strictFP modes only as a 1st step, so we're not
causing
> perf regressions that require reverting large patches.
> >
> >
> > The patch is already 3 weeks in the tree. PR51556 is the only problem
so
> far and it is a missed optimization, not malfunction. Besides, as it is
> said in the ticket, "the SLP call vectorization fails as we don't
properly
> account for calls that have different return/arg types", so the
problem
> already existed, this intrinsic only elucidated it. I think if in a couple
> of weeks no new problems will be found, work in this direction can be
> continued, as if there were something broken, it would be already reported.
> >>
> >>
> >> On Tue, Aug 24, 2021 at 8:26 AM Kevin Neal <Kevin.Neal at
sas.com> wrote:
> >>>
> >>> It probably doesn’t help that IEEE 754-2019 uses the word
“signal” to
> mean, paraphrased, “set a bit in a status register and continue producing a
> result with no trap”, and that’s in the default FP environment. So IEEE
> “signaling” is not Unix “signaling”.
> >>>
> >>>
> >>>
> >>> Once one picks up on that distinction the IEEE 754 document
reads a
> lot like Andy’s description of x86-64 behavior.
> >>>
> >>>
> >>>
> >>> From: Kaylor, Andrew <andrew.kaylor at intel.com>
> >>> Sent: Monday, August 23, 2021 6:31 PM
> >>> To: Arthur O'Dwyer <arthur.j.odwyer at gmail.com>;
Sanjay Patel <
> spatel at rotateright.com>
> >>> Cc: Kevin Neal <Kevin.Neal at sas.com>; Wang, Pengfei
<
> pengfei.wang at intel.com>; llvm-dev at lists.llvm.org; Clang Dev <
> cfe-dev at lists.llvm.org>
> >>> Subject: RE: [cfe-dev] [llvm-dev] Intrinsic llvm::isnan
> >>>
> >>>
> >>>
> >>> EXTERNAL
> >>>
> >>> I think the confusion here has to do with what it means for
the
> optimizer to assume that “floating-point exceptions will be masked”. What
> I’m about to say may be specific to the x86-64-based processor behavior
> because that’s the one I know, but I think other architectures will have
> similar behavior but possibly with different terms?
> >>>
> >>>
> >>>
> >>> If a floating point exception occurs in an operation and that
> exception is masked in the floating point control registers, the status bit
> for that exception will be set and nothing else will happen. If a floating
> point exception occurs in an operation and that exception is NOT masked in
> the floating point control registers, an exception handler will be called.
> >>>
> >>>
> >>>
> >>> So, what the current documentation means to say is that the
optimizer
> may assume that the status flags will not be read and that no exception
> handler will be called if a floating point exception occurs. Basically, it
> means that floating point exceptions will be ignored in all ways.
> >>>
> >>>
> >>>
> >>>
> >>>
> >>> > It's still not clear to me if there's a benefit
from having an
> intrinsic vs. one more exception mode ("none" or
"off").
> >>>
> >>>
> >>>
> >>> The key point to recognize here is that the metadata arguments
to the
> constrained intrinsics are intended to describe assumptions that can or
> cannot be made. They are not intended to bring about the state that they
> describe. So, for example, if the rounding mode argument is set to
> “fpround.tonearest” that means the optimizer can assume that the processor
> is set to use the “tonearest” rounding mode, but it does not enforce this.
> If we see this flag during ISel, we can select an instruction that takes an
> explicit rounding mode argument and use the “tonearest” value for that
> argument, but we are also allowed to select an instruction that uses the
> rounding mode from the MXCSR register and assume that when this instruction
> is executed that rounding mode will be “tonearest”.
> >>>
> >>>
> >>>
> >>> If you need an intrinsic that is guaranteed not to raise
exceptions,
> that should be a distinct intrinsic from any similar intrinsic that may
> raise exceptions. See, for example, llvm.experimental.constrained.nearbyint
> and llvm.experimental.constrained.rint which differ only in exception
> behavior.
> >>>
> >>>
> >>>
> >>>
> >>>
> >>> -Andy
> >>>
> >>>
> >>>
> >>>
> >>>
> >>> From: Arthur O'Dwyer <arthur.j.odwyer at gmail.com>
> >>> Sent: Monday, August 23, 2021 2:37 PM
> >>> To: Sanjay Patel <spatel at rotateright.com>
> >>> Cc: Kaylor, Andrew <andrew.kaylor at intel.com>; Kevin
Neal <
> Kevin.Neal at sas.com>; Wang, Pengfei <pengfei.wang at intel.com>;
> llvm-dev at lists.llvm.org; Clang Dev <cfe-dev at lists.llvm.org>
> >>> Subject: Re: [cfe-dev] [llvm-dev] Intrinsic llvm::isnan
> >>>
> >>>
> >>>
> >>> On Mon, Aug 23, 2021 at 3:42 PM Sanjay Patel via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
> >>>
> >>> Ok, does this edit to the LangRef make sense for the
definition of
> "ignore":
> >>>
> >>> "optimization passes may assume that the exception status
flags will
> not be read and that floating-point exceptions **will** be masked"
-->
> >>>
> >>> "optimization passes may assume that the exception status
flags will
> not be read and that floating-point exceptions **may** be masked"
> >>>
> >>>
> >>>
> >>> I haven't been following the technical details, but in
terms of the
> English documentation, it makes no sense to say that someone "may
assume
> that [X] may happen." Either [X] always happens, in which case
optimization
> passes may safely assume that it happens; or [X] never happens, in which
> case optimization passes may safely assume that it does not happen; or else
> [X] sometimes happens and sometimes doesn't, in which case
optimizations
> passes must not assume anything about [X].
> >>>
> >>>
> >>>
> >>> So you might say: "optimization passes may assume that
the exception
> status flags will not be read. Floating-point exceptions might or might not
> be masked, depending on [____]" (and then mention the relevant
variable,
> such as "instruction set" or "optimization level" or
whatever).
> >>>
> >>>
> >>>
> >>> HTH,
> >>>
> >>> Arthur
> >>
> >> _______________________________________________
> >> cfe-dev mailing list
> >> cfe-dev at lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210825/7cb4ac92/attachment-0001.html>

On Tue, Aug 24, 2021 at 1:53 PM Roman Lebedev via cfe-dev <
cfe-dev at lists.llvm.org> wrote:
> Regardless of everything, i would like to see a patch that restores
> the -ffast-math handling, and *then* the RFC on what the is-nan check
> should do when -ffast-math is present.
> It is more than possible that the RFC will succeed,
> but i don't think a change like that should happen the way it did.

I find the rationale to be convincing, as to the need for a change. But,
the scope of the proposal is too narrow. We cannot discuss fast-math
semantics changes *only* for "isnan", it needs to be in the context of
the
desired behavior for all operations -- the RFC should cover the entire set
of changes we want to eventually make, even if isnan is the only thing
implemented so far. Discussing this greater scope could result in a
different desired implementation, rather than simply adding
"llvm.isnan"
intrinsic.

Yet, even with that expanded scope, the two halves of the proposal are
still going to be closely linked, so I suspect it still makes sense to
discuss both the strict-fp and fast-math changes in a single RFC.

Anyhow, for the fast-math section, I believe the proposed semantics ought
to be:
  The -ffinite-math-only and -fno-signed-zeros options do not impact the
ability to accurately load, store, copy, or pass or return such values from
general function calls. They also do not impact any of the
"non-computational" and "quiet-computational" IEEE-754
operations, which
includes classification functions (fpclassify, signbit, isinf/isnan/etc),
sign-modification (copysign, fabs, and negation `-(x)`), as well as
the totalorder and totalordermag functions. Those correctly handle NaN,
Inf, and signed zeros even when the flags are in effect. These flags
*do* affect
the behavior of other expressions and math standard-library calls, as well
as comparison operations.

I would not expect this to have an actual negative impact on the
performance benefit of those flags, since the optimization benefits mainly
arise from comparisons and the general computation instructions which are
unchanged.

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949 was mentioned as
> motivational, but i don't see any resolution there,
> it's not even in "confirmed" state.

I agree, this is not at all clear evidence as to GCC's position on the
matter.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210824/6bf95a8f/attachment.html>