[PATCH] D110322: [ConstantFolding] Fold constrained compare intrinsics

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added a comment.

In D110322#3159399 <https://reviews.llvm.org/D110322#3159399>, @craig.topper wrote:

> Should the change that effects fadd, fma, etc. tests be moved to a different patch that is a pre-requisite of the compare change?

Yes, it is a good idea to move this debatable change into a separate patch, it is here: D114766 <https://reviews.llvm.org/D114766>. This patch does not depend on it.



================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:2293
+  FCmpInst::Predicate Cond = FCmp->getPredicate();
+  if (IntrinsicID == Intrinsic::experimental_constrained_fcmp) {
+    if (Op1.isSignaling() || Op2.isSignaling())
----------------
craig.topper wrote:
> Does ConstrainedFPCmpIntrinsic have any method for determining fcmp vs fcmps? If not should it?
Probably, but it does not help here. The interface is designed to evaluate constant value without construction of a node. ConstrainedFPCmpIntrinsic can be extracted from `Call` but checking intrinsic ID seems simpler.


================
Comment at: llvm/lib/IR/Instructions.cpp:4243
 
+bool FCmpInst::evaluate(Predicate Pred, const APFloat &Op1,
+                        const APFloat &Op2) {
----------------
craig.topper wrote:
> This feels a little like it shouldn't be part of FCmpInst, but I don't have a concrete suggestion of where to put it instead. ConstantFold.cpp feels like the right home, but unfortunatley the header for that isn't visible to ConstantFolding.cpp.
I moved ConstantFold to `include` directory in dependency patch and put `evaluatePredicate` there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110322