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

[David Kreitzer via Phabricator via llvm-commits]

DavidKreitzer added a comment.

This all looks pretty good to me, Andy. Regarding

> In https://reviews.llvm.org/D27028#634214, @b-sumner wrote:
> 
>> Also, I don't see any reason to cover frem, but I also don't understand how frem ever got to be an instruction instead of a standard C library intrinsic.
> 
> 
> I'm not sure it actually happens, but I would think that frem is potentially subject to the same sorts of constant folding that can be done with fdiv.

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".

Regarding the denormal-handling issue, I agree that it is reasonable to defer support to a subsequent patch. I suspect you are ultimately going to want a separate argument rather than folding denormal-handling into the rounding mode. But we can discuss that in the Bugzilla report that you opened.



================
Comment at: docs/LangRef.rst:12147
+
+'``llvm.experimental.constrained.fsub``' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
----------------
This question might be beyond the scope of this initial change set, but here goes.

FP negation is currently handled using fsub -0, X. Is that sufficient in a constrained context? If we allow X to be a signaling NaN, -0-X should raise Invalid while -X should not, at least according to IEEE-754.



================
Comment at: lib/IR/Verifier.cpp:4265
+  StringRef RoundingArg = dyn_cast<MDString>(RoundingMD)->getString();
+  Assert(RoundingArg.equals("round.dynamic") ||
+             RoundingArg.equals("round.tonearest") ||
----------------
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? 


Repository:
  rL LLVM

https://reviews.llvm.org/D27028