[InstCombine] Skip division sinking with allowReciprocal flag

[jinpzhanAMD]

When the allowReciprocal fast-math flag is set, avoid reassociating (X / Y) * Z into (X * Z) / Y.
The original form (X / Y) * Z can be optimized by the backend using reciprocal instructions as X * rcp(Y) * Z, which may be more efficient than transforming it to (X * Z) / Y. Sinking the division would:

1. Change the computation order, potentially affecting precision
2. Prevent the backend from using efficient rcp instructions
3. Miss opportunities for FMA or other specialized instructions

This change prevents InstCombine from interfering with backend-level
reciprocal optimizations and maintains the intended computation order.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: None (jinpzhanAMD)

Changes

When the allowReciprocal fast-math flag is set, avoid reassociating (X / Y) * Z into (X * Z) / Y.
The original form (X / Y) * Z can be optimized by the backend using reciprocal instructions as X * rcp(Y) * Z, which may be more efficient than transforming it to (X * Z) / Y. Sinking the division would:

1. Change the computation order, potentially affecting precision
2. Prevent the backend from using efficient rcp instructions
3. Miss opportunities for FMA or other specialized instructions

This change prevents InstCombine from interfering with backend-level
reciprocal optimizations and maintains the intended computation order.

---

Full diff: https://github.com/llvm/llvm-project/pull/167633.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp (+8-1)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
index a9aacc707cc20..88700af60d633 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
@@ -795,7 +795,8 @@ Instruction *InstCombinerImpl::foldFMulReassoc(BinaryOperator &I) {
         return BinaryOperator::CreateFDivFMF(CC1, X, FMF);
     }
     if (match(Op0, m_FDiv(m_Value(X), m_Constant(C1)))) {
-      // FIXME: This seems like it should also be checking for arcp
+      // Constant folding is not affected by allowReciprocal,
+      // since constants are calculated at compile time.
       // (X / C1) * C --> X * (C / C1)
       Constant *CDivC1 =
           ConstantFoldBinaryOpOperands(Instruction::FDiv, C, C1, DL);
@@ -837,6 +838,12 @@ Instruction *InstCombinerImpl::foldFMulReassoc(BinaryOperator &I) {
                      m_Value(Z)))) {
     BinaryOperator *DivOp = cast<BinaryOperator>(((Z == Op0) ? Op1 : Op0));
     FastMathFlags FMF = I.getFastMathFlags() & DivOp->getFastMathFlags();
+    // Preserve (X / Y) * Z when reciprocal math is enabled to allow backend
+    // rcp instruction emission. Sinking to (X * Z) / Y would prevent this
+    // optimization, alter computation order and potentially affecting
+    // precision.
+    if (FMF.allowReciprocal())
+      return nullptr;
     if (FMF.allowReassoc()) {
       // Sink division: (X / Y) * Z --> (X * Z) / Y
       auto *NewFMul = Builder.CreateFMulFMF(X, Z, FMF);

[arsenm]

Missing tests

[arsenm]

This isn't exactly how semantics works

[jcranmer-intel]

Also, despite the original comment, this really needs to be matching against the reassoc flag, not the arcp flag (since arcp explicitly doesn't allow reassociation of the division operation, only the inversion of the division).

[jcranmer-intel]

1. Change the computation order, potentially affecting precision

If reassoc flag is enabled, the user is explicitly letting us change the computation order. (We do have some issues with the canonical reassociation interrupting thinks like FMA formation, but that's a somewhat orthogonal issue).

2. Prevent the backend from using efficient rcp instructions

This is unclear to me--(X * Z) / Y seems to me to be just as easily be turned into (X * Z) * rcp(Y) as the original (X / Y) * Z can be turned into (X * rcp(Y)) * Z. What's the actual hangup?

[jinpzhanAMD]

Thanks for the reviews.
Reassoc flag and the arcp flag are two independent (orthogonal) fmf flags, which shouldn't depend on each other.
The precision issue I'm working on can be fixed from our internal repo, thus I would close this PR.