[llvm] r209049 - Add support for combining GEPs across PHI nodes
Louis Gerbarg
lgg at apple.com
Fri May 16 19:17:03 PDT 2014
I'll take a look in a few minutes, but I am not at all familiar with the sanitizer.
Louis
> On May 16, 2014, at 6:08 PM, Eric Christopher <echristo at gmail.com> wrote:
>
> Think this might be causing the current sanitizer failures on the
> bots. Could you take a look?
>
> -eric
>
>> On Fri, May 16, 2014 at 4:47 PM, Louis Gerbarg <lgg at apple.com> wrote:
>> Author: louis
>> Date: Fri May 16 18:47:24 2014
>> New Revision: 209049
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=209049&view=rev
>> Log:
>> Add support for combining GEPs across PHI nodes
>>
>> Currently LLVM will generally merge GEPs. This allows backends to use more
>> complex addressing modes. In some cases this is not happening because there
>> is PHI inbetween the two GEPs:
>>
>> GEP1--\
>> |-->PHI1-->GEP3
>> GEP2--/
>>
>> This patch checks to see if GEP1 and GEP2 are similiar enough that they can be
>> cloned (GEP12) in GEP3's BB, allowing GEP->GEP merging (GEP123):
>>
>> GEP1--\ --\ --\
>> |-->PHI1-->GEP3 ==> |-->PHI2->GEP12->GEP3 == > |-->PHI2->GEP123
>> GEP2--/ --/ --/
>>
>> This also breaks certain use chains that are preventing GEP->GEP merges that the
>> the existing instcombine would merge otherwise.
>>
>> Tests included.
>>
>> rdar://15547484
>>
>> Added:
>> 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=209049&r1=209048&r2=209049&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Fri May 16 18:47:24 2014
>> @@ -1220,6 +1220,65 @@ 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() != Op1->getNumOperands())
>> + return nullptr;
>> +
>> + for (unsigned J = 0, F = Op1->getNumOperands(); J != F; ++J) {
>> + 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.
>> + PHINode *NewPN = Builder->CreatePHI(Op1->getOperand(DI)->getType(),
>> + PN->getNumOperands());
>> + 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.
>>
>> Added: llvm/trunk/test/Transforms/InstCombine/gepphigep.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/gepphigep.ll?rev=209049&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/InstCombine/gepphigep.ll (added)
>> +++ llvm/trunk/test/Transforms/InstCombine/gepphigep.ll Fri May 16 18:47:24 2014
>> @@ -0,0 +1,56 @@
>> +; 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
More information about the llvm-commits
mailing list