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

[Serge Pavlov via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:1407
   switch (F->getIntrinsicID()) {
   case Intrinsic::fabs:
   case Intrinsic::minnum:
----------------
andrew.w.kaylor wrote:
> sepavloff wrote:
> > evandro wrote:
> > > Should these cases be true even when `isStrictFP()` is true?
> > `fabs` is allowed irrespective of `isStrictFP()` , so it should be processed here. As for functions like `ceil`, they cannot be found in strictfp function, corresponding operations are represented by constrained intrinsics.
> I originally added the Call->isStrictFP() check here before we had constrained versions of the primary FP intrinsics. Now that we have them it should be OK to let those pass. However, we aren't yet enforcing the rule that all calls within a strictfp function must be the constrained versions, so it might be too soon to make that assumption here.
> 
> Also, I don't think we're planning to have constrained versions of the target specific intrinsics like Intrinsic::x86_avx512_vcvtss2usi32. Those will probably only have the strictfp attribute on the callsite and possibly an operand bundle to specify the specific details. The other intrinsics might revert to behavior like that too some day.
Reorganized this function.


================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:1822
+      if (U.isFinite()) {
+        U.roundToIntegral(*RM);
+      } else if (U.isSignaling()) {
----------------
andrew.w.kaylor wrote:
> For all of these except nearbyint, you need to check the return code and only remove the call if no flags are raised or exceptionBehavior is ignore. The rest of the functions raise the inexact flag if the value is not already an integer.
> 
> We could both keep the call and RAUW the return value. We need the exception to be raised, but the constant folding could unlock additional optimizations. It might be useful to introduce new intrinsics like llvm.raiseinexact() so that we could later combine multiple calls raising the same flag.
> For all of these except nearbyint, you need to check the return code
None of rounding functions raises exception, if argument is not SNaN. All big numbers are already integers, so overflow cannot occur. Result of rounding is integer number, so underflow or inexact exceptions also absent. Mentioning of inexact exception in the description of `rint` in C standard is somewhat misleading, it is discusses earlier: https://reviews.llvm.org/D72930#inline-676354.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72930