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

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff marked 2 inline comments as done.
sepavloff 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;
----------------
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.


================
Comment at: llvm/lib/IR/FPEnv.cpp:82
+  case fp::rmDynamic: return None;
+  case fp::rmToNearest: return APFloat::rmNearestTiesToAway;
+  case fp::rmDownward: return APFloat::rmTowardNegative;
----------------
craig.topper wrote:
> sepavloff wrote:
> > craig.topper wrote:
> > > rmToNearest should map to APFloat::rmNearestTiesToEven.
> > > rmToNearest should map to APFloat::rmNearestTiesToEven
> > 
> > Why? It seems that the C11 standard (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf) does not specify which of two two IEEE-754 rounding modes corresponds to 'nearest'. Description of `round` however contains definite requirement (7.12.9.6):
> > ```
> > The round functions round their argument to the nearest integer value in floating-point
> > format, rounding halfway cases away from zero, regardless of the current rounding
> > direction.
> > ```
> > In this case rounding mode is `roundTiesToAway`. Why in other cases it must be `roundTiesToEven`?
> As far as I know the default environment is supposed to be TiesToEven.  That's what all of the constant folding for non-strict FP assumes. The round function itself is weird and specifies a different behavior.
Yes, I see that ConstantFolding.cpp uses mainly `roundTiesToEven` and will change the patch. It would be nice to understand why this is so.

What make me worry is unusual behavior of `roundTiesToEven`. For instance, 10.5 is rounded to 10.0. Behavior of `round` is more familiar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72930