[llvm] Update foldFMulReassoc to respect absent fast-math flags (PR #88589)
Andy Kaylor via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 16 16:09:34 PDT 2024
================
@@ -631,31 +631,38 @@ Instruction *InstCombinerImpl::foldFMulReassoc(BinaryOperator &I) {
Value *Op1 = I.getOperand(1);
Value *X, *Y;
Constant *C;
+ BinaryOperator *Op0BinOp;
// Reassociate constant RHS with another constant to form constant
// expression.
- if (match(Op1, m_Constant(C)) && C->isFiniteNonZeroFP()) {
+ if (match(Op1, m_Constant(C)) && C->isFiniteNonZeroFP() &&
+ match(Op0, m_AllowReassoc(m_BinOp(Op0BinOp)))) {
+ // Everything in this scope folds I with Op0, intersecting their FMF.
+ FastMathFlags FMF = I.getFastMathFlags() & Op0BinOp->getFastMathFlags();
+ IRBuilder<>::FastMathFlagGuard FMFGuard(Builder);
+ Builder.setFastMathFlags(FMF);
Constant *C1;
if (match(Op0, m_OneUse(m_FDiv(m_Constant(C1), m_Value(X))))) {
// (C1 / X) * C --> (C * C1) / X
Constant *CC1 =
ConstantFoldBinaryOpOperands(Instruction::FMul, C, C1, DL);
if (CC1 && CC1->isNormalFP())
- return BinaryOperator::CreateFDivFMF(CC1, X, &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
----------------
andykaylor wrote:
My thinking is that division is not associative, and you can only apply the associative property if you replace division with multiplication by the reciprocal. I agree that we're not consistent about this, but I don't see how skipping the arcp check can be justified.
FWIW, gcc thinks this requires reciprocal math: https://godbolt.org/z/T7xTTshbf
https://github.com/llvm/llvm-project/pull/88589
More information about the llvm-commits
mailing list