[PATCH] D27028: Add intrinsics for constrained floating point operations

[Andy Kaylor via Phabricator via llvm-commits]

andrew.w.kaylor added a comment.

In https://reviews.llvm.org/D27028#635372, @DavidKreitzer wrote:

> When frem has a meaningful result, it is always exact, so perhaps we should omit the rounding behavior argument? I think we still need a constrained frem intrinsic, though, to handle the exceptional behavior in cases such as "frem inf, x" and "frem x, 0".


The implementation is slightly simpler if I give the frem intrinsic a rounding argument, even though it isn't needed.  Otherwise, that intrinsic couldn't be handled by the same subclass as the others.  I realize that's a fairly weak argument, but I just feel like making this one intrinsic different from the others will make the code ugly.



================
Comment at: lib/IR/Verifier.cpp:4265
+  StringRef RoundingArg = dyn_cast<MDString>(RoundingMD)->getString();
+  Assert(RoundingArg.equals("round.dynamic") ||
+             RoundingArg.equals("round.tonearest") ||
----------------
DavidKreitzer wrote:
> Rather than duplicating the legal rounding mode & exception behavior strings here (and in the getRoundingMode & getExceptionBehavior methods), would it be better to have a string-->enum function that is called in both places? 
I do like the idea of having a single location for these strings.  On the other hand, I think I'd need to introduce a new enum value (invalid?) that is only used here.  I'll think about it a bit and try to consolidate the strings one way or another.


Repository:
  rL LLVM

https://reviews.llvm.org/D27028