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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:2299
+  APFloat::opStatus St = APFloat::opOK;
+  auto FCmp = cast<ConstrainedFPCmpIntrinsic>(Call);
+  FCmpInst::Predicate Cond = FCmp->getPredicate();
----------------
`auto *FCmp` I believe LLVM coding standards prefer to keep the `*` around with auto so things that are pointers are obvious.


================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:2293
+  FCmpInst::Predicate Cond = FCmp->getPredicate();
+  if (IntrinsicID == Intrinsic::experimental_constrained_fcmp) {
+    if (Op1.isSignaling() || Op2.isSignaling())
----------------
nikic wrote:
> sepavloff wrote:
> > 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.
> FYI recently `ICmpInst::compare()` was added for the corresponding operation on ICmpInsts (in https://github.com/llvm/llvm-project/commit/25043c8276644e684f8d14cd4cadaa87a7e99b0e), so it might make sense to have the same method on FCmpInst. (No strong opinion on placement though.)
The ConstrainedFPCmpIntrinsic is already being extracted from the call to get the predicate. So I thought it would make sense to get the signaling state from it as well and not pass the intrinsic ID at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110322