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

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 5 14:25:41 PDT 2020


craig.topper added inline comments.


================
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),
----------------
RKSimon wrote:
> sepavloff wrote:
> > 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.
> I'm OK with this being dropped - @craig.topper @kpn ?
I'm fine removing it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74729



More information about the llvm-commits mailing list