[llvm] r363813 - [Reassociate] Handle unary FNeg in the Reassociate pass
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 20 14:13:25 PDT 2019
Hi Cameron,
This patch breaks some of internal testing. I filed https://bugs.llvm.org/show_bug.cgi?id=42349 <https://bugs.llvm.org/show_bug.cgi?id=42349> for a reproducer.
Basically we hit the assertion that FNeg doesn’t have the fast flag. So either the assertion is bogus or we miss a place that needs to check the flag is here.
I let you check.
Cheers,
-Quentin
> On Jun 19, 2019, at 7:59 AM, Cameron McInally via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Author: mcinally
> Date: Wed Jun 19 07:59:14 2019
> New Revision: 363813
>
> URL: http://llvm.org/viewvc/llvm-project?rev=363813&view=rev
> Log:
> [Reassociate] Handle unary FNeg in the Reassociate pass
>
> Differential Revision: https://reviews.llvm.org/D63445
>
> Modified:
> llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp
> llvm/trunk/test/Transforms/Reassociate/fast-basictest.ll
>
> Modified: llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp?rev=363813&r1=363812&r2=363813&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp Wed Jun 19 07:59:14 2019
> @@ -266,12 +266,20 @@ static BinaryOperator *CreateNeg(Value *
>
> /// Replace 0-X with X*-1.
> static BinaryOperator *LowerNegateToMultiply(Instruction *Neg) {
> + assert((isa<UnaryOperator>(Neg) || isa<BinaryOperator>(Neg)) &&
> + "Expected a Negate!");
> + // It's not safe to lower a unary FNeg into a FMul by -1.0. However,
> + // we can only reach this function with fast flags set, so it's
> + // safe to do with nnan.
> + assert((!isa<FPMathOperator>(Neg) || Neg->isFast()) &&
> + "Expecting FastMathFlags!");
> + unsigned OpNo = isa<BinaryOperator>(Neg) ? 1 : 0;
> Type *Ty = Neg->getType();
> Constant *NegOne = Ty->isIntOrIntVectorTy() ?
> ConstantInt::getAllOnesValue(Ty) : ConstantFP::get(Ty, -1.0);
>
> - BinaryOperator *Res = CreateMul(Neg->getOperand(1), NegOne, "", Neg, Neg);
> - Neg->setOperand(1, Constant::getNullValue(Ty)); // Drop use of op.
> + BinaryOperator *Res = CreateMul(Neg->getOperand(OpNo), NegOne, "", Neg, Neg);
> + Neg->setOperand(OpNo, Constant::getNullValue(Ty)); // Drop use of op.
> Res->takeName(Neg);
> Neg->replaceAllUsesWith(Res);
> Res->setDebugLoc(Neg->getDebugLoc());
> @@ -444,8 +452,10 @@ using RepeatedValue = std::pair<Value*,
> /// that have all uses inside the expression (i.e. only used by non-leaf nodes
> /// of the expression) if it can turn them into binary operators of the right
> /// type and thus make the expression bigger.
> -static bool LinearizeExprTree(BinaryOperator *I,
> +static bool LinearizeExprTree(Instruction *I,
> SmallVectorImpl<RepeatedValue> &Ops) {
> + assert((isa<UnaryOperator>(I) || isa<BinaryOperator>(I)) &&
> + "Expected a UnaryOperator or BinaryOperator!");
> LLVM_DEBUG(dbgs() << "LINEARIZE: " << *I << '\n');
> unsigned Bitwidth = I->getType()->getScalarType()->getPrimitiveSizeInBits();
> unsigned Opcode = I->getOpcode();
> @@ -462,7 +472,7 @@ static bool LinearizeExprTree(BinaryOper
> // with their weights, representing a certain number of paths to the operator.
> // If an operator occurs in the worklist multiple times then we found multiple
> // ways to get to it.
> - SmallVector<std::pair<BinaryOperator*, APInt>, 8> Worklist; // (Op, Weight)
> + SmallVector<std::pair<Instruction*, APInt>, 8> Worklist; // (Op, Weight)
> Worklist.push_back(std::make_pair(I, APInt(Bitwidth, 1)));
> bool Changed = false;
>
> @@ -489,10 +499,10 @@ static bool LinearizeExprTree(BinaryOper
> SmallPtrSet<Value *, 8> Visited; // For sanity checking the iteration scheme.
> #endif
> while (!Worklist.empty()) {
> - std::pair<BinaryOperator*, APInt> P = Worklist.pop_back_val();
> + std::pair<Instruction*, APInt> P = Worklist.pop_back_val();
> I = P.first; // We examine the operands of this binary operator.
>
> - for (unsigned OpIdx = 0; OpIdx < 2; ++OpIdx) { // Visit operands.
> + for (unsigned OpIdx = 0; OpIdx < I->getNumOperands(); ++OpIdx) { // Visit operands.
> Value *Op = I->getOperand(OpIdx);
> APInt Weight = P.second; // Number of paths to this operand.
> LLVM_DEBUG(dbgs() << "OPERAND: " << *Op << " (" << Weight << ")\n");
> @@ -572,14 +582,14 @@ static bool LinearizeExprTree(BinaryOper
>
> // If this is a multiply expression, turn any internal negations into
> // multiplies by -1 so they can be reassociated.
> - if (BinaryOperator *BO = dyn_cast<BinaryOperator>(Op))
> - if ((Opcode == Instruction::Mul && match(BO, m_Neg(m_Value()))) ||
> - (Opcode == Instruction::FMul && match(BO, m_FNeg(m_Value())))) {
> + if (Instruction *Tmp = dyn_cast<Instruction>(Op))
> + if ((Opcode == Instruction::Mul && match(Tmp, m_Neg(m_Value()))) ||
> + (Opcode == Instruction::FMul && match(Tmp, m_FNeg(m_Value())))) {
> LLVM_DEBUG(dbgs()
> << "MORPH LEAF: " << *Op << " (" << Weight << ") TO ");
> - BO = LowerNegateToMultiply(BO);
> - LLVM_DEBUG(dbgs() << *BO << '\n');
> - Worklist.push_back(std::make_pair(BO, Weight));
> + Tmp = LowerNegateToMultiply(Tmp);
> + LLVM_DEBUG(dbgs() << *Tmp << '\n');
> + Worklist.push_back(std::make_pair(Tmp, Weight));
> Changed = true;
> continue;
> }
> @@ -2020,7 +2030,7 @@ Instruction *ReassociatePass::canonicali
> /// instructions is not allowed.
> void ReassociatePass::OptimizeInst(Instruction *I) {
> // Only consider operations that we understand.
> - if (!isa<BinaryOperator>(I))
> + if (!isa<UnaryOperator>(I) && !isa<BinaryOperator>(I))
> return;
>
> if (I->getOpcode() == Instruction::Shl && isa<ConstantInt>(I->getOperand(1)))
> @@ -2085,7 +2095,8 @@ void ReassociatePass::OptimizeInst(Instr
> I = NI;
> }
> }
> - } else if (I->getOpcode() == Instruction::FSub) {
> + } else if (I->getOpcode() == Instruction::FNeg ||
> + I->getOpcode() == Instruction::FSub) {
> if (ShouldBreakUpSubtract(I)) {
> Instruction *NI = BreakUpSubtract(I, RedoInsts);
> RedoInsts.insert(I);
> @@ -2094,7 +2105,9 @@ void ReassociatePass::OptimizeInst(Instr
> } else if (match(I, m_FNeg(m_Value()))) {
> // Otherwise, this is a negation. See if the operand is a multiply tree
> // and if this is not an inner node of a multiply tree.
> - if (isReassociableOp(I->getOperand(1), Instruction::FMul) &&
> + Value *Op = isa<BinaryOperator>(I) ? I->getOperand(1) :
> + I->getOperand(0);
> + if (isReassociableOp(Op, Instruction::FMul) &&
> (!I->hasOneUse() ||
> !isReassociableOp(I->user_back(), Instruction::FMul))) {
> // If the negate was simplified, revisit the users to see if we can
>
> Modified: llvm/trunk/test/Transforms/Reassociate/fast-basictest.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Reassociate/fast-basictest.ll?rev=363813&r1=363812&r2=363813&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/Reassociate/fast-basictest.ll (original)
> +++ llvm/trunk/test/Transforms/Reassociate/fast-basictest.ll Wed Jun 19 07:59:14 2019
> @@ -586,12 +586,10 @@ define float @test18(float %a, float %b,
> ret float %f
> }
>
> -; FIXME: This reassociation is not working.
> define float @test18_unary_fneg(float %a, float %b, float %z) {
> ; CHECK-LABEL: @test18_unary_fneg(
> -; CHECK-NEXT: [[C:%.*]] = fmul fast float [[Z:%.*]], -4.000000e+01
> -; CHECK-NEXT: [[E:%.*]] = fmul fast float [[C]], [[A:%.*]]
> -; CHECK-NEXT: [[F:%.*]] = fneg fast float [[E]]
> +; CHECK-NEXT: [[E:%.*]] = fmul fast float [[A:%.*]], 4.000000e+01
> +; CHECK-NEXT: [[F:%.*]] = fmul fast float [[E]], [[Z:%.*]]
> ; CHECK-NEXT: ret float [[F]]
> ;
> %d = fmul fast float %z, 4.000000e+01
> @@ -616,6 +614,21 @@ define float @test18_reassoc(float %a, f
> ret float %f
> }
>
> +; It is not safe to reassociate unary fneg without nnan.
> +define float @test18_reassoc_unary_fneg(float %a, float %b, float %z) {
> +; CHECK-LABEL: @test18_reassoc_unary_fneg(
> +; CHECK-NEXT: [[C:%.*]] = fmul reassoc float [[Z:%.*]], -4.000000e+01
> +; CHECK-NEXT: [[E:%.*]] = fmul reassoc float [[C]], [[A:%.*]]
> +; CHECK-NEXT: [[F:%.*]] = fneg reassoc float [[E]]
> +; CHECK-NEXT: ret float [[F]]
> +;
> + %d = fmul reassoc float %z, 4.000000e+01
> + %c = fneg reassoc float %d
> + %e = fmul reassoc float %a, %c
> + %f = fneg reassoc float %e
> + ret float %f
> +}
> +
> ; With sub reassociation, constant folding can eliminate the 12 and -12 constants.
> define float @test19(float %A, float %B) {
> ; CHECK-LABEL: @test19(
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190620/471f6963/attachment.html>
More information about the llvm-commits
mailing list