[PATCH] D72930: [FEnv] Constfold some unary constrained operations

Kevin P. Neal via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 12 05:45:48 PST 2020


kpn added inline comments.


================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:1773
+    if (IntrinsicID == Intrinsic::experimental_constrained_nearbyint ||
+        IntrinsicID == Intrinsic::experimental_constrained_rint) {
+      APFloat::roundingMode RM = APFloat::rmNearestTiesToEven;
----------------
sepavloff wrote:
> kpn wrote:
> > sepavloff wrote:
> > > kpn wrote:
> > > > I thought rint could raise an inexact exception?
> > > > 
> > > > Also, what happens if we don't know the floating point environment because of FENV_ACCESS=ON and no other math flags or math #pragmas have been given? Shouldn't the infrastructure for that go into clang first?
> > > `rint` **may** raise the inexact floating-point exception but does not have to do it. Result of rounding never requires rounding, so it is always exact. See http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_291.htm for related ideas.
> > > 
> > > If we don't know rounding mode, the corresponding intrinsic should have "round.dynamic" as argument, `getAPFloatRoundingMode` returns `None` in this case and the expression is not folded.
> > > 
> > > Yes, it is clang that must put "round.dynamic". In D69272 there is a test that checks such behavior.
> > But the latest C2x standard still has the same description of rint from C99. The standard hasn't changed. So, are we allowed to simply drop an exception? I thought we weren't?
> > 
> > What's the definition of "may"? Is it:
> > 1) If conditions warrant, an exception _shall_ be issued OR
> > 2) If conditions warrant, the decision to raise an exception is _implementation defined_.
> > 
> > If it's #1 then we're dropping an exception we should be issuing. 
> `Inexact` exception rises when result of an operation cannot be represented without loss of precision. Result of any rounding operation is an integer, it always can be represented without loss of significant digits. The reason why `rint` exists is faster implementation than `nearbyint` in some cases. I don't know these particular cases but the cited defect report mentions it.
> 
> So `rint` is just a loose version of `nearbyint`. There are no cases when this exception would make sense.
After sleeping on it I agree. Having rint() raise Inexact in the exact circumstance where you needed to call rint() anyway isn't helpful. So I'd guess that this is case #2 above and we can elide the exception.

Objection withdrawn, and sorry I wasn't quicker to come to this conclusion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72930/new/

https://reviews.llvm.org/D72930





More information about the llvm-commits mailing list