[PATCH] D74729: [FPEnv] Intrinsic for setting rounding mode

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff marked 2 inline comments as done.
sepavloff added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6197
+    Res = DAG.getNode(ISD::SET_ROUNDING, sdl, MVT::Other,
+                      {DAG.getRoot(), getValue(I.getArgOperand(0))});
+    setValue(&I, Res);
----------------
craig.topper wrote:
> Don't you need to call getRoot not DAG.getRoot()?
Indeed. Thank you!


================
Comment at: llvm/lib/IR/Verifier.cpp:4973
+  case Intrinsic::set_rounding: {
+    if (auto RM = dyn_cast<ConstantInt>(Call.getArgOperand(0))) {
+      Assert(RM->getZExtValue() < static_cast<unsigned>(RoundingMode::Dynamic),
----------------
craig.topper wrote:
> Is the argument intended to always be a constant or we're just verifying it when we can? The latter seems unusual.
The argument may be a variable. If it is a constant, it must be a valid rounding mode. It is expected to be a value of type `RoundingMode`. Values from 0 to 4 denote IEEE rounding modes, they may be followed by target-specific rounding modes. The argument value must be less than `RoundingMode::Dynamic`, which now if 7.

I am hesitating if this code is useful enough, as even for constant argument its validity cannot be verified due to non-IEEE rounding modes. Probably we should remove this check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74729