spatel added a comment. In D104854#2957582 <https://reviews.llvm.org/D104854#2957582>, @lebedev.ri wrote:
> In D104854#2957529 <https://reviews.llvm.org/D104854#2957529>, @spatel wrote: > >> In D104854#2957471 <https://reviews.llvm.org/D104854#2957471>, @sepavloff >> wrote: >> >>> In D104854#2957423 <https://reviews.llvm.org/D104854#2957423>, @spatel >>> wrote: >>> >>>> Is it intentional that we are not canonicalizing the intrinsic call back >>>> to `fcmp uno` in the default FP environment? >>> >>> It is lowered to unordered comparison by default. Changing `llvm.isnan` to >>> `fcmp uno` somewhere in IR would make it possible to optimize out the >>> latter if fast-math mode is on. Preserving semantics of `isnan` when >>> fast-math is in effect was one of the goals of this change. >> >> I understand that the codegen was supposed to be no worse, but the >> difference in IR causes optimizer regressions like: >> https://llvm.org/PR51556 >> >> If we want this intrinsic (and its siblings that haven't been created yet) >> to survive through IR, then we have to enhance IR passes to recognize the >> new patterns. >> It would be easier to do this in steps: (1) create the intrinsic only if not >> in the default FP env, (2) update IR analysis/passes to recognize the >> intrinsic, (3) create the intrinsic in the default FP env with no FMF, (4) >> create the intrinsic always. > > +1, but right now i'm not sold on the behavior of not optimizing away NaN > checks in no-NaN's mode. > At least that part should be reconciled now. It *might* be an improvement, > but it caters to expectations > of one group while catering away from the documentation and existing > expectations of other groups. > This shouldn't be decided in a review, it should be driven by an RFC. Agree. I think a revert followed by RFC to make sure there is consensus on semantics is needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104854/new/ https://reviews.llvm.org/D104854 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
