[llvm] r278938 - Revert "Reassociate: Reprocess RedoInsts after each inst".
Hans Wennborg via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 17 15:21:13 PDT 2016
Merged in r278993. Thanks!
Do you want to check in the test case from PR28367 so we don't regress
that in the future?
On Wed, Aug 17, 2016 at 10:32 AM, <mcrosier at codeaurora.org> wrote:
> 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