[llvm] r278938 - Revert "Reassociate: Reprocess RedoInsts after each inst".
via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 18 06:40:01 PDT 2016
On 2016-08-17 18:21, Hans Wennborg wrote:
> Merged in r278993. Thanks!
>
> Do you want to check in the test case from PR28367 so we don't regress
> that in the future?
Sure. Done in r279063. Thanks, Hans.
> 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