[PATCH] D126190: [InstCombine] Add combine for fcmp sqrt(x),C --> fcmp x,C*C

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 23 11:28:32 PDT 2022


spatel added inline comments.


================
Comment at: llvm/include/llvm/IR/PatternMatch.h:2204
+template <typename Opnd0>
+inline typename m_Intrinsic_Ty<Opnd0>::Ty m_Sqrt(const Opnd0 &Op0) {
+  return m_Intrinsic<Intrinsic::sqrt>(Op0);
----------------
This is useful already, so I added it as a preliminary step:
e8c20d995bed


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:6760
+    const APFloat *CF;
+    if (match(Op1, m_APFloat(CF)) && !CF->isNegative() && I.isFast()) {
+      Constant *C = ConstantFP::get(X->getType(), *CF);
----------------
We're (very slowly) moving away from using FMF with fcmp because fcmp doesn't usually have a logical relationship to the FMF. 
What really matters for this fold is that we can't require precise results from sqrt, so we should check that the sqrt has "reassoc" or "afn".


================
Comment at: llvm/test/Transforms/InstCombine/fcmp.ll:1234
+  %sqrt = call double @llvm.sqrt.f64(double %v)
+  %cmp = fcmp fast ogt double %sqrt, -2.000000e+00
+  ret i1 %cmp
----------------
Can we handle this kind of example as a preliminary patch? There's no fast-math required if we are checking if the result of sqrt is or is not negative (but we should test several predicates to verify that we have the correct behavior with NAN):
https://alive2.llvm.org/ce/z/EeYGLt
https://alive2.llvm.org/ce/z/X7qsg2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126190



More information about the llvm-commits mailing list