Serge Pavlov via llvm-dev
2021-Aug-24 17:33 UTC
[llvm-dev] [cfe-dev] Intrinsic llvm::isnan
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> <https://reviews.llvm.org/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 >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210825/2ddfe0fc/attachment.html>
Roman Lebedev via llvm-dev
2021-Aug-24 17:53 UTC
[llvm-dev] [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