[llvm] [InstCombine] Transform high latency, dependent FSQRT/FDIV into FMUL (PR #87474)

via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 10 22:12:39 PDT 2024


================
@@ -626,6 +626,127 @@ Instruction *InstCombinerImpl::foldPowiReassoc(BinaryOperator &I) {
   return nullptr;
 }
 
+bool isFSqrtDivToFMulLegal(Instruction *X, SmallSetVector<Instruction *, 2> &R1,
+                           SmallSetVector<Instruction *, 2> &R2) {
+
+  BasicBlock *BBx = X->getParent();
+  BasicBlock *BBr1 = R1[0]->getParent();
+  BasicBlock *BBr2 = R2[0]->getParent();
+
+  auto IsStrictFP = [](Instruction *I) {
+    IntrinsicInst *II = dyn_cast<IntrinsicInst>(I);
+    return II && II->isStrictFP();
+  };
+
+  // Check the constaints on instruction X.
+  auto XConstraintsSatisfied = [X, &IsStrictFP]() {
+    if (IsStrictFP(X))
+      return false;
+    // X must atleast have 4 uses.
+    // 3 uses as part of
+    //    r1 = x * x
+    //    r2 = a * x
+    // Now, post-transform, r1/r2 will no longer have usage of 'x' and if the
+    // changes to 'x' need to persist, we must have one more usage of 'x'
+    if (!X->hasNUsesOrMore(4))
+      return false;
+    // Check if reciprocalFP is enabled.
+    bool RecipFPMath = dyn_cast<FPMathOperator>(X)->hasAllowReciprocal();
----------------
sushgokh wrote:

> `1.0 / sqrt(a)` is _not_ the same as `sqrt(a) / a`--it is for real numbers, but FP numbers are not real numbers--so converting `1.0 / sqrt(a)` to `sqrt(a) / a` requires a fast-math flag of some kind, and the one that we've been abusing here is the `reassoc` flag, which has kind of devolved in practice into "permit algebraic rewrites".

So, do you suggest that  `1.0 / sqrt(a)` to `sqrt(a) / a` requires reassoc flag and then convert `sqrt(a) / a` to `sqrt(a) * 1/a`   under arcp flag?
If my understanding is correct, this would require combination of `reassoc arcp`, right?

https://github.com/llvm/llvm-project/pull/87474


More information about the llvm-commits mailing list