[llvm] r209067 - Revert r209049 and r209065, "Add support for combining GEPs across PHI nodes"
Louis Gerbarg
lgg at apple.com
Sat May 17 19:07:15 PDT 2014
Thanks. I'll look at the reduction and see what I can come up with.
Louis
Sent from my iPhone
> On May 17, 2014, at 11:43 AM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>
> Thanks! I was just in the process of reducing a testcase for it. A
> testcase is attached.
>
> It should crash with
>
> opt -instcombine test2.bc
>
>> On 17 May 2014 10:39, NAKAMURA Takumi <geek4civic at gmail.com> wrote:
>> Author: chapuni
>> Date: Sat May 17 09:39:21 2014
>> New Revision: 209067
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=209067&view=rev
>> Log:
>> Revert r209049 and r209065, "Add support for combining GEPs across PHI nodes"
>>
>> It broke clang selfhosting even after r209065.
>>
>> Removed:
>> llvm/trunk/test/Transforms/InstCombine/gepphigep.ll
>> Modified:
>> llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=209067&r1=209066&r2=209067&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Sat May 17 09:39:21 2014
>> @@ -1220,72 +1220,6 @@ Instruction *InstCombiner::visitGetEleme
>> if (MadeChange) return &GEP;
>> }
>>
>> - // Check to see if the inputs to the PHI node are getelementptr instructions.
>> - if (PHINode *PN = dyn_cast<PHINode>(PtrOp)) {
>> - GetElementPtrInst *Op1 = dyn_cast<GetElementPtrInst>(PN->getOperand(0));
>> - if (!Op1)
>> - return nullptr;
>> -
>> - signed DI = -1;
>> -
>> - for (auto I = PN->op_begin()+1, E = PN->op_end(); I !=E; ++I) {
>> - GetElementPtrInst *Op2 = dyn_cast<GetElementPtrInst>(*I);
>> - if (!Op2 || Op1->getNumOperands() != Op2->getNumOperands())
>> - return nullptr;
>> -
>> - for (unsigned J = 0, F = Op1->getNumOperands(); J != F; ++J) {
>> - if (Op1->getOperand(J)->getType() != Op2->getOperand(J)->getType())
>> - return nullptr;
>> -
>> - if (Op1->getOperand(J) != Op2->getOperand(J)) {
>> - if (DI == -1) {
>> - // We have not seen any differences yet in the GEPs feeding the
>> - // PHI yet, so we record this one.
>> - DI = J;
>> - } else {
>> - // The GEP is different by more than one input. While this could be
>> - // extended to support GEPs that vary by more than one variable it
>> - // doesn't make sense since it greatly increases the complexity and
>> - // would result in an R+R+R addressing mode which no backend
>> - // directly supports and would need to be broken into several
>> - // simpler instructions anyway.
>> - return nullptr;
>> - }
>> - }
>> - }
>> - }
>> -
>> - GetElementPtrInst *NewGEP = dyn_cast<GetElementPtrInst>(Op1->clone());
>> -
>> - if (DI == -1) {
>> - // All the GEPs feeding the PHI are identical. Clone one down into our
>> - // BB so that it can be merged with the current GEP.
>> - GEP.getParent()->getInstList().insert(GEP.getParent()->getFirstNonPHI(),
>> - NewGEP);
>> - } else {
>> - // All the GEPs feeding the PHI differ at a single offset. Clone a GEP
>> - // into the current block so it can be merged, and create a new PHI to
>> - // set that index.
>> - Instruction *InsertPt = Builder->GetInsertPoint();
>> - Builder->SetInsertPoint(PN);
>> - PHINode *NewPN = Builder->CreatePHI(Op1->getOperand(DI)->getType(),
>> - PN->getNumOperands());
>> - Builder->SetInsertPoint(InsertPt);
>> -
>> - for (auto &I : PN->operands())
>> - NewPN->addIncoming(dyn_cast<GEPOperator>(I)->getOperand(DI),
>> - PN->getIncomingBlock(I));
>> -
>> - NewGEP->setOperand(DI, NewPN);
>> - GEP.getParent()->getInstList().insert(GEP.getParent()->getFirstNonPHI(),
>> - NewGEP);
>> - NewGEP->setOperand(DI, NewPN);
>> - }
>> -
>> - GEP.setOperand(0, NewGEP);
>> - PtrOp = NewGEP;
>> - }
>> -
>> // Combine Indices - If the source pointer to this getelementptr instruction
>> // is a getelementptr instruction, combine the indices of the two
>> // getelementptr instructions into a single instruction.
>>
>> Removed: llvm/trunk/test/Transforms/InstCombine/gepphigep.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/gepphigep.ll?rev=209066&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/InstCombine/gepphigep.ll (original)
>> +++ llvm/trunk/test/Transforms/InstCombine/gepphigep.ll (removed)
>> @@ -1,56 +0,0 @@
>> -; RUN: opt -instcombine -S < %s | FileCheck %s
>> -
>> -%struct1 = type { %struct2*, i32, i32, i32 }
>> -%struct2 = type { i32, i32 }
>> -
>> -define i32 @test1(%struct1* %dm, i1 %tmp4, i64 %tmp9, i64 %tmp19) {
>> -bb:
>> - %tmp = getelementptr inbounds %struct1* %dm, i64 0, i32 0
>> - %tmp1 = load %struct2** %tmp, align 8
>> - br i1 %tmp4, label %bb1, label %bb2
>> -
>> -bb1:
>> - %tmp10 = getelementptr inbounds %struct2* %tmp1, i64 %tmp9
>> - %tmp11 = getelementptr inbounds %struct2* %tmp10, i64 0, i32 0
>> - store i32 0, i32* %tmp11, align 4
>> - br label %bb3
>> -
>> -bb2:
>> - %tmp20 = getelementptr inbounds %struct2* %tmp1, i64 %tmp19
>> - %tmp21 = getelementptr inbounds %struct2* %tmp20, i64 0, i32 0
>> - store i32 0, i32* %tmp21, align 4
>> - br label %bb3
>> -
>> -bb3:
>> - %phi = phi %struct2* [ %tmp10, %bb1 ], [ %tmp20, %bb2 ]
>> - %tmp24 = getelementptr inbounds %struct2* %phi, i64 0, i32 1
>> - %tmp25 = load i32* %tmp24, align 4
>> - ret i32 %tmp25
>> -
>> -; CHECK-LABEL: @test1(
>> -; CHECK: getelementptr inbounds %struct2* %tmp1, i64 %tmp9, i32 0
>> -; CHECK: getelementptr inbounds %struct2* %tmp1, i64 %tmp19, i32 0
>> -; CHECK: %[[PHI:[0-9A-Za-z]+]] = phi i64 [ %tmp9, %bb1 ], [ %tmp19, %bb2 ]
>> -; CHECK: getelementptr inbounds %struct2* %tmp1, i64 %[[PHI]], i32 1
>> -}
>> -
>> -define i32 @test2(%struct1* %dm, i1 %tmp4, i64 %tmp9, i64 %tmp19) {
>> -bb:
>> - %tmp = getelementptr inbounds %struct1* %dm, i64 0, i32 0
>> - %tmp1 = load %struct2** %tmp, align 8
>> - %tmp10 = getelementptr inbounds %struct2* %tmp1, i64 %tmp9
>> - %tmp11 = getelementptr inbounds %struct2* %tmp10, i64 0, i32 0
>> - store i32 0, i32* %tmp11, align 4
>> - %tmp20 = getelementptr inbounds %struct2* %tmp1, i64 %tmp19
>> - %tmp21 = getelementptr inbounds %struct2* %tmp20, i64 0, i32 0
>> - store i32 0, i32* %tmp21, align 4
>> - %tmp24 = getelementptr inbounds %struct2* %tmp10, i64 0, i32 1
>> - %tmp25 = load i32* %tmp24, align 4
>> - ret i32 %tmp25
>> -
>> -; CHECK-LABEL: @test2(
>> -; CHECK: getelementptr inbounds %struct2* %tmp1, i64 %tmp9, i32 0
>> -; CHECK: getelementptr inbounds %struct2* %tmp1, i64 %tmp19, i32 0
>> -; CHECK: getelementptr inbounds %struct2* %tmp1, i64 %tmp9, i32 1
>> -}
>> -
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> <test2.bc>
More information about the llvm-commits
mailing list