[llvm] r209067 - Revert r209049 and r209065, "Add support for combining GEPs across PHI nodes"

Rafael EspĂ­ndola rafael.espindola at gmail.com
Sat May 17 11:43:29 PDT 2014


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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test2.bc
Type: application/octet-stream
Size: 44284 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140517/070b4994/attachment.obj>


More information about the llvm-commits mailing list