[llvm] r278938 - Revert "Reassociate: Reprocess RedoInsts after each inst".
via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 17 10:32:21 PDT 2016
Hans,
Would you mind cherry-picking this commit to the 3.9 release branch?
This should address PR28367.
Chad
On 2016-08-17 11:54, Chad Rosier via llvm-commits wrote:
> Author: mcrosier
> Date: Wed Aug 17 10:54:39 2016
> New Revision: 278938
>
> URL: http://llvm.org/viewvc/llvm-project?rev=278938&view=rev
> Log:
> Revert "Reassociate: Reprocess RedoInsts after each inst".
>
> This reverts commit r258830, which introduced a bug described in
> PR28367.
>
> PR28367
>
> Removed:
> llvm/trunk/test/Transforms/Reassociate/prev_insts_canonicalized.ll
> Modified:
> llvm/trunk/include/llvm/Transforms/Scalar/Reassociate.h
> llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp
>
> llvm/trunk/test/Transforms/Reassociate/reassoc-intermediate-fnegs.ll
> llvm/trunk/test/Transforms/Reassociate/xor_reassoc.ll
>
> Modified: llvm/trunk/include/llvm/Transforms/Scalar/Reassociate.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Scalar/Reassociate.h?rev=278938&r1=278937&r2=278938&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Transforms/Scalar/Reassociate.h (original)
> +++ llvm/trunk/include/llvm/Transforms/Scalar/Reassociate.h Wed Aug 17
> 10:54:39 2016
> @@ -65,7 +65,7 @@ public:
> PreservedAnalyses run(Function &F, FunctionAnalysisManager &);
>
> private:
> - void BuildRankMap(Function &F, ReversePostOrderTraversal<Function *>
> &RPOT);
> + void BuildRankMap(Function &F);
> unsigned getRank(Value *V);
> void canonicalizeOperands(Instruction *I);
> void ReassociateExpression(BinaryOperator *I);
>
> Modified: llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp?rev=278938&r1=278937&r2=278938&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp Wed Aug 17
> 10:54:39 2016
> @@ -145,8 +145,7 @@ static BinaryOperator *isReassociableOp(
> return nullptr;
> }
>
> -void ReassociatePass::BuildRankMap(
> - Function &F, ReversePostOrderTraversal<Function *> &RPOT) {
> +void ReassociatePass::BuildRankMap(Function &F) {
> unsigned i = 2;
>
> // Assign distinct ranks to function arguments.
> @@ -155,6 +154,7 @@ void ReassociatePass::BuildRankMap(
> DEBUG(dbgs() << "Calculated Rank[" << I->getName() << "] = " << i
> << "\n");
> }
>
> + ReversePostOrderTraversal<Function *> RPOT(&F);
> for (BasicBlock *BB : RPOT) {
> unsigned BBRank = RankMap[BB] = ++i << 16;
>
> @@ -2172,28 +2172,13 @@ void ReassociatePass::ReassociateExpress
> }
>
> PreservedAnalyses ReassociatePass::run(Function &F,
> FunctionAnalysisManager &) {
> - // Reassociate needs for each instruction to have its operands
> already
> - // processed, so we first perform a RPOT of the basic blocks so that
> - // when we process a basic block, all its dominators have been
> processed
> - // before.
> - ReversePostOrderTraversal<Function *> RPOT(&F);
> - BuildRankMap(F, RPOT);
> + // Calculate the rank map for F.
> + BuildRankMap(F);
>
> MadeChange = false;
> - for (BasicBlock *BI : RPOT) {
> - // Use a worklist to keep track of which instructions have been
> processed
> - // (and which insts won't be optimized again) so when redoing
> insts,
> - // optimize insts rightaway which won't be processed later.
> - SmallSet<Instruction *, 8> Worklist;
> -
> - // Insert all instructions in the BB
> - for (Instruction &I : *BI)
> - Worklist.insert(&I);
> -
> + for (Function::iterator BI = F.begin(), BE = F.end(); BI != BE;
> ++BI) {
> // Optimize every instruction in the basic block.
> - for (BasicBlock::iterator II = BI->begin(), IE = BI->end(); II !=
> IE;) {
> - // This instruction has been processed.
> - Worklist.erase(&*II);
> + for (BasicBlock::iterator II = BI->begin(), IE = BI->end(); II !=
> IE;)
> if (isInstructionTriviallyDead(&*II)) {
> EraseInst(&*II++);
> } else {
> @@ -2202,22 +2187,27 @@ PreservedAnalyses ReassociatePass::run(F
> ++II;
> }
>
> - // If the above optimizations produced new instructions to
> optimize or
> - // made modifications which need to be redone, do them now if
> they won't
> - // be handled later.
> - while (!RedoInsts.empty()) {
> - Instruction *I = RedoInsts.pop_back_val();
> - // Process instructions that won't be processed later, either
> - // inside the block itself or in another basic block (based on
> rank),
> - // since these will be processed later.
> - if ((I->getParent() != BI || !Worklist.count(I)) &&
> - RankMap[I->getParent()] <= RankMap[BI]) {
> - if (isInstructionTriviallyDead(I))
> - EraseInst(I);
> - else
> - OptimizeInst(I);
> - }
> - }
> + // Make a copy of all the instructions to be redone so we can
> remove dead
> + // instructions.
> + SetVector<AssertingVH<Instruction>> ToRedo(RedoInsts);
> + // Iterate over all instructions to be reevaluated and remove
> trivially dead
> + // instructions. If any operand of the trivially dead instruction
> becomes
> + // dead mark it for deletion as well. Continue this process until
> all
> + // trivially dead instructions have been removed.
> + while (!ToRedo.empty()) {
> + Instruction *I = ToRedo.pop_back_val();
> + if (isInstructionTriviallyDead(I))
> + RecursivelyEraseDeadInsts(I, ToRedo);
> + }
> +
> + // Now that we have removed dead instructions, we can reoptimize
> the
> + // remaining instructions.
> + while (!RedoInsts.empty()) {
> + Instruction *I = RedoInsts.pop_back_val();
> + if (isInstructionTriviallyDead(I))
> + EraseInst(I);
> + else
> + OptimizeInst(I);
> }
> }
>
>
> Removed:
> llvm/trunk/test/Transforms/Reassociate/prev_insts_canonicalized.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Reassociate/prev_insts_canonicalized.ll?rev=278937&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/Reassociate/prev_insts_canonicalized.ll
> (original)
> +++ llvm/trunk/test/Transforms/Reassociate/prev_insts_canonicalized.ll
> (removed)
> @@ -1,57 +0,0 @@
> -; RUN: opt < %s -reassociate -S | FileCheck %s
> -
> -; These tests make sure that before processing insts
> -; any previous instructions are already canonicalized.
> -define i32 @foo(i32 %in) {
> -; CHECK-LABEL: @foo
> -; CHECK-NEXT: %factor = mul i32 %in, -4
> -; CHECK-NEXT: %factor1 = mul i32 %in, 2
> -; CHECK-NEXT: %_3 = add i32 %factor, 1
> -; CHECK-NEXT: %_5 = add i32 %_3, %factor1
> -; CHECK-NEXT: ret i32 %_5
> - %_0 = add i32 %in, 1
> - %_1 = mul i32 %in, -2
> - %_2 = add i32 %_0, %_1
> - %_3 = add i32 %_1, %_2
> - %_4 = add i32 %_3, 1
> - %_5 = add i32 %in, %_3
> - ret i32 %_5
> -}
> -
> -; CHECK-LABEL: @foo1
> -define void @foo1(float %in, i1 %cmp) {
> -wrapper_entry:
> - br label %foo1
> -
> -for.body:
> - %0 = fadd float %in1, %in1
> - br label %foo1
> -
> -foo1:
> - %_0 = fmul fast float %in, -3.000000e+00
> - %_1 = fmul fast float %_0, 3.000000e+00
> - %in1 = fadd fast float -3.000000e+00, %_1
> - %in1use = fadd fast float %in1, %in1
> - br label %for.body
> -
> -
> -}
> -
> -; CHECK-LABEL: @foo2
> -define void @foo2(float %in, i1 %cmp) {
> -wrapper_entry:
> - br label %for.body
> -
> -for.body:
> -; If the operands of the phi are sheduled for processing before
> -; foo1 is processed, the invariant of reassociate are not preserved
> - %unused = phi float [%in1, %foo1], [undef, %wrapper_entry]
> - br label %foo1
> -
> -foo1:
> - %_0 = fmul fast float %in, -3.000000e+00
> - %_1 = fmul fast float %_0, 3.000000e+00
> - %in1 = fadd fast float -3.000000e+00, %_1
> - %in1use = fadd fast float %in1, %in1
> - br label %for.body
> -}
>
> Modified:
> llvm/trunk/test/Transforms/Reassociate/reassoc-intermediate-fnegs.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Reassociate/reassoc-intermediate-fnegs.ll?rev=278938&r1=278937&r2=278938&view=diff
> ==============================================================================
> ---
> llvm/trunk/test/Transforms/Reassociate/reassoc-intermediate-fnegs.ll
> (original)
> +++
> llvm/trunk/test/Transforms/Reassociate/reassoc-intermediate-fnegs.ll
> Wed Aug 17 10:54:39 2016
> @@ -1,8 +1,8 @@
> ; RUN: opt < %s -reassociate -S | FileCheck %s
> ; CHECK-LABEL: faddsubAssoc1
> -; CHECK: [[TMP1:%.*]] = fsub fast half 0xH8000, %a
> -; CHECK: [[TMP2:%.*]] = fadd fast half %b, [[TMP1]]
> -; CHECK: fmul fast half [[TMP2]], 0xH4500
> +; CHECK: [[TMP1:%tmp.*]] = fmul fast half %a, 0xH4500
> +; CHECK: [[TMP2:%tmp.*]] = fmul fast half %b, 0xH4500
> +; CHECK: fsub fast half [[TMP2]], [[TMP1]]
> ; CHECK: ret
> ; Input is A op (B op C)
> define half @faddsubAssoc1(half %a, half %b) {
>
> Modified: llvm/trunk/test/Transforms/Reassociate/xor_reassoc.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Reassociate/xor_reassoc.ll?rev=278938&r1=278937&r2=278938&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/Reassociate/xor_reassoc.ll (original)
> +++ llvm/trunk/test/Transforms/Reassociate/xor_reassoc.ll Wed Aug 17
> 10:54:39 2016
> @@ -88,8 +88,8 @@ define i32 @xor_special2(i32 %x, i32 %y)
> %xor1 = xor i32 %xor, %and
> ret i32 %xor1
> ; CHECK-LABEL: @xor_special2(
> -; CHECK: %xor = xor i32 %y, 123
> -; CHECK: %xor1 = xor i32 %xor, %x
> +; CHECK: %xor = xor i32 %x, 123
> +; CHECK: %xor1 = xor i32 %xor, %y
> ; CHECK: ret i32 %xor1
> }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list