From ed1cb01baf1727ce2eb9c58deafe1a92d6fc65b7 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 13 May 2022 12:48:52 +0200 Subject: [PATCH] [IRBuilder] Add IsInBounds parameter to CreateGEP() We commonly want to create either an inbounds or non-inbounds GEP based on a boolean value, e.g. when preserving inbounds from existing GEPs. Directly accept such a boolean in the API, rather than requiring a ternary between CreateGEP and CreateInBoundsGEP. This change is not entirely NFC, because we now preserve an inbounds flag in a constant expression edge-case in InstCombine. --- llvm/include/llvm/IR/IRBuilder.h | 13 +++-- llvm/lib/CodeGen/CodeGenPrepare.cpp | 14 ++--- llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp | 13 ++--- .../InstCombine/InstructionCombining.cpp | 58 ++++++------------- llvm/lib/Transforms/Scalar/SROA.cpp | 20 ++----- .../Scalar/StraightLineStrengthReduce.cpp | 18 ++---- .../Transforms/Vectorize/LoopVectorize.cpp | 7 +-- .../InstCombine/gep-combine-loop-invariant.ll | 2 +- 8 files changed, 48 insertions(+), 97 deletions(-) diff --git a/llvm/include/llvm/IR/IRBuilder.h b/llvm/include/llvm/IR/IRBuilder.h index e97cbf8aae82..b8fb30362a05 100644 --- a/llvm/include/llvm/IR/IRBuilder.h +++ b/llvm/include/llvm/IR/IRBuilder.h @@ -1723,17 +1723,18 @@ public: } Value *CreateGEP(Type *Ty, Value *Ptr, ArrayRef IdxList, - const Twine &Name = "") { - if (auto *V = Folder.FoldGEP(Ty, Ptr, IdxList, /*IsInBounds=*/false)) + const Twine &Name = "", bool IsInBounds = false) { + if (auto *V = Folder.FoldGEP(Ty, Ptr, IdxList, IsInBounds)) return V; - return Insert(GetElementPtrInst::Create(Ty, Ptr, IdxList), Name); + return Insert(IsInBounds + ? GetElementPtrInst::CreateInBounds(Ty, Ptr, IdxList) + : GetElementPtrInst::Create(Ty, Ptr, IdxList), + Name); } Value *CreateInBoundsGEP(Type *Ty, Value *Ptr, ArrayRef IdxList, const Twine &Name = "") { - if (auto *V = Folder.FoldGEP(Ty, Ptr, IdxList, /*IsInBounds=*/true)) - return V; - return Insert(GetElementPtrInst::CreateInBounds(Ty, Ptr, IdxList), Name); + return CreateGEP(Ty, Ptr, IdxList, Name, /* IsInBounds */ true); } Value *CreateConstGEP1_32(Type *Ty, Value *Ptr, unsigned Idx0, diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp index c91dfd254a65..d1000383c872 100644 --- a/llvm/lib/CodeGen/CodeGenPrepare.cpp +++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp @@ -5324,11 +5324,8 @@ bool CodeGenPrepare::optimizeMemoryInst(Instruction *MemoryInst, Value *Addr, // SDAG consecutive load/store merging. if (ResultPtr->getType() != I8PtrTy) ResultPtr = Builder.CreatePointerCast(ResultPtr, I8PtrTy); - ResultPtr = - AddrMode.InBounds - ? Builder.CreateInBoundsGEP(I8Ty, ResultPtr, ResultIndex, - "sunkaddr") - : Builder.CreateGEP(I8Ty, ResultPtr, ResultIndex, "sunkaddr"); + ResultPtr = Builder.CreateGEP(I8Ty, ResultPtr, ResultIndex, + "sunkaddr", AddrMode.InBounds); } ResultIndex = V; @@ -5339,11 +5336,8 @@ bool CodeGenPrepare::optimizeMemoryInst(Instruction *MemoryInst, Value *Addr, } else { if (ResultPtr->getType() != I8PtrTy) ResultPtr = Builder.CreatePointerCast(ResultPtr, I8PtrTy); - SunkAddr = - AddrMode.InBounds - ? Builder.CreateInBoundsGEP(I8Ty, ResultPtr, ResultIndex, - "sunkaddr") - : Builder.CreateGEP(I8Ty, ResultPtr, ResultIndex, "sunkaddr"); + SunkAddr = Builder.CreateGEP(I8Ty, ResultPtr, ResultIndex, "sunkaddr", + AddrMode.InBounds); } if (SunkAddr->getType() != Addr->getType()) diff --git a/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp b/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp index 888fc8ffac2c..44afdb8d8ec1 100644 --- a/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp +++ b/llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp @@ -278,15 +278,10 @@ Value *GenericToNVVM::remapConstantExpr(Module *M, Function *F, ConstantExpr *C, C->getIndices()); case Instruction::GetElementPtr: // GetElementPtrConstantExpr - return cast(C)->isInBounds() - ? Builder.CreateGEP( - cast(C)->getSourceElementType(), - NewOperands[0], - makeArrayRef(&NewOperands[1], NumOperands - 1)) - : Builder.CreateInBoundsGEP( - cast(C)->getSourceElementType(), - NewOperands[0], - makeArrayRef(&NewOperands[1], NumOperands - 1)); + return Builder.CreateGEP(cast(C)->getSourceElementType(), + NewOperands[0], + makeArrayRef(&NewOperands[1], NumOperands - 1), "", + cast(C)->isInBounds()); case Instruction::Select: // SelectConstantExpr return Builder.CreateSelect(NewOperands[0], NewOperands[1], NewOperands[2]); diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 6124f7035b5d..4fc6b4de976b 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1943,10 +1943,8 @@ static Instruction *foldSelectGEP(GetElementPtrInst &GEP, SmallVector IndexC(GEP.indices()); bool IsInBounds = GEP.isInBounds(); Type *Ty = GEP.getSourceElementType(); - Value *NewTrueC = IsInBounds ? Builder.CreateInBoundsGEP(Ty, TrueC, IndexC) - : Builder.CreateGEP(Ty, TrueC, IndexC); - Value *NewFalseC = IsInBounds ? Builder.CreateInBoundsGEP(Ty, FalseC, IndexC) - : Builder.CreateGEP(Ty, FalseC, IndexC); + Value *NewTrueC = Builder.CreateGEP(Ty, TrueC, IndexC, "", IsInBounds); + Value *NewFalseC = Builder.CreateGEP(Ty, FalseC, IndexC, "", IsInBounds); return SelectInst::Create(Cond, NewTrueC, NewFalseC, "", nullptr, Sel); } @@ -2000,11 +1998,9 @@ Instruction *InstCombinerImpl::visitGEPOfGEP(GetElementPtrInst &GEP, // -- have to recreate %src & %gep // put NewSrc at same location as %src Builder.SetInsertPoint(cast(Src)); - Value *NewSrc = Builder.CreateGEP( - GEP.getSourceElementType(), SO0, GO1, Src->getName()); - // Propagate 'inbounds' if the new source was not constant-folded. - if (auto *NewSrcGEPI = dyn_cast(NewSrc)) - NewSrcGEPI->setIsInBounds(Src->isInBounds()); + Value *NewSrc = + Builder.CreateGEP(GEP.getSourceElementType(), SO0, GO1, + Src->getName(), Src->isInBounds()); GetElementPtrInst *NewGEP = GetElementPtrInst::Create( GEP.getSourceElementType(), NewSrc, {SO1}); NewGEP->setIsInBounds(GEP.isInBounds()); @@ -2178,9 +2174,8 @@ Instruction *InstCombinerImpl::visitGEPOfBitcast(BitCastInst *BCI, // existing GEP Value. Causing issues if this Value is accessed when // constructing an AddrSpaceCastInst SmallVector Indices(GEP.indices()); - Value *NGEP = GEP.isInBounds() - ? Builder.CreateInBoundsGEP(SrcEltType, SrcOp, Indices) - : Builder.CreateGEP(SrcEltType, SrcOp, Indices); + Value *NGEP = + Builder.CreateGEP(SrcEltType, SrcOp, Indices, "", GEP.isInBounds()); NGEP->takeName(&GEP); // Preserve GEP address space to satisfy users @@ -2231,12 +2226,10 @@ Instruction *InstCombinerImpl::visitGEPOfBitcast(BitCastInst *BCI, // Otherwise, if the offset is non-zero, we need to find out if there is a // field at Offset in 'A's type. If so, we can pull the cast through the // GEP. - SmallVector NewIndices; + SmallVector NewIndices; if (findElementAtOffset(SrcType, Offset.getSExtValue(), NewIndices, DL)) { - Value *NGEP = - GEP.isInBounds() - ? Builder.CreateInBoundsGEP(SrcEltType, SrcOp, NewIndices) - : Builder.CreateGEP(SrcEltType, SrcOp, NewIndices); + Value *NGEP = Builder.CreateGEP(SrcEltType, SrcOp, NewIndices, "", + GEP.isInBounds()); if (NGEP->getType() == GEP.getType()) return replaceInstUsesWith(GEP, NGEP); @@ -2539,11 +2532,8 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) { // addrspacecast i8 addrspace(1)* %0 to i8* SmallVector Idx(GEP.indices()); Value *NewGEP = - GEP.isInBounds() - ? Builder.CreateInBoundsGEP(StrippedPtrEltTy, StrippedPtr, - Idx, GEP.getName()) - : Builder.CreateGEP(StrippedPtrEltTy, StrippedPtr, Idx, - GEP.getName()); + Builder.CreateGEP(StrippedPtrEltTy, StrippedPtr, Idx, + GEP.getName(), GEP.isInBounds()); return new AddrSpaceCastInst(NewGEP, GEPType); } } @@ -2558,13 +2548,9 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) { DL.getTypeAllocSize(StrippedPtrEltTy->getArrayElementType()) == DL.getTypeAllocSize(GEPEltType)) { Type *IdxType = DL.getIndexType(GEPType); - Value *Idx[2] = { Constant::getNullValue(IdxType), GEP.getOperand(1) }; - Value *NewGEP = - GEP.isInBounds() - ? Builder.CreateInBoundsGEP(StrippedPtrEltTy, StrippedPtr, Idx, - GEP.getName()) - : Builder.CreateGEP(StrippedPtrEltTy, StrippedPtr, Idx, - GEP.getName()); + Value *Idx[2] = {Constant::getNullValue(IdxType), GEP.getOperand(1)}; + Value *NewGEP = Builder.CreateGEP(StrippedPtrEltTy, StrippedPtr, Idx, + GEP.getName(), GEP.isInBounds()); // V and GEP are both pointer types --> BitCast return CastInst::CreatePointerBitCastOrAddrSpaceCast(NewGEP, GEPType); @@ -2596,11 +2582,8 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) { // If the multiplication NewIdx * Scale may overflow then the new // GEP may not be "inbounds". Value *NewGEP = - GEP.isInBounds() && NSW - ? Builder.CreateInBoundsGEP(StrippedPtrEltTy, StrippedPtr, - NewIdx, GEP.getName()) - : Builder.CreateGEP(StrippedPtrEltTy, StrippedPtr, NewIdx, - GEP.getName()); + Builder.CreateGEP(StrippedPtrEltTy, StrippedPtr, NewIdx, + GEP.getName(), GEP.isInBounds() && NSW); // The NewGEP must be pointer typed, so must the old one -> BitCast return CastInst::CreatePointerBitCastOrAddrSpaceCast(NewGEP, @@ -2641,11 +2624,8 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) { Value *Off[2] = {Constant::getNullValue(IndTy), NewIdx}; Value *NewGEP = - GEP.isInBounds() && NSW - ? Builder.CreateInBoundsGEP(StrippedPtrEltTy, StrippedPtr, - Off, GEP.getName()) - : Builder.CreateGEP(StrippedPtrEltTy, StrippedPtr, Off, - GEP.getName()); + Builder.CreateGEP(StrippedPtrEltTy, StrippedPtr, Off, + GEP.getName(), GEP.isInBounds() && NSW); // The NewGEP must be pointer typed, so must the old one -> BitCast return CastInst::CreatePointerBitCastOrAddrSpaceCast(NewGEP, GEPType); diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp index 1f68c3734f64..a1c335650dff 100644 --- a/llvm/lib/Transforms/Scalar/SROA.cpp +++ b/llvm/lib/Transforms/Scalar/SROA.cpp @@ -3474,19 +3474,13 @@ private: Type *Ty = GEPI.getSourceElementType(); Value *True = Sel->getTrueValue(); - Value *NTrue = - IsInBounds - ? IRB.CreateInBoundsGEP(Ty, True, Index, - True->getName() + ".sroa.gep") - : IRB.CreateGEP(Ty, True, Index, True->getName() + ".sroa.gep"); + Value *NTrue = IRB.CreateGEP(Ty, True, Index, True->getName() + ".sroa.gep", + IsInBounds); Value *False = Sel->getFalseValue(); - Value *NFalse = - IsInBounds - ? IRB.CreateInBoundsGEP(Ty, False, Index, - False->getName() + ".sroa.gep") - : IRB.CreateGEP(Ty, False, Index, False->getName() + ".sroa.gep"); + Value *NFalse = IRB.CreateGEP(Ty, False, Index, + False->getName() + ".sroa.gep", IsInBounds); Value *NSel = IRB.CreateSelect(Sel->getCondition(), NTrue, NFalse, Sel->getName() + ".sroa.sel"); @@ -3540,10 +3534,8 @@ private: IRB.SetInsertPoint(In->getParent(), std::next(In->getIterator())); Type *Ty = GEPI.getSourceElementType(); - NewVal = IsInBounds ? IRB.CreateInBoundsGEP(Ty, In, Index, - In->getName() + ".sroa.gep") - : IRB.CreateGEP(Ty, In, Index, - In->getName() + ".sroa.gep"); + NewVal = IRB.CreateGEP(Ty, In, Index, In->getName() + ".sroa.gep", + IsInBounds); } NewPN->addIncoming(NewVal, B); } diff --git a/llvm/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp index a093e98a8503..70df0cec0dca 100644 --- a/llvm/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp +++ b/llvm/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp @@ -682,24 +682,16 @@ void StraightLineStrengthReduce::rewriteCandidateWithBasis( unsigned AS = Basis.Ins->getType()->getPointerAddressSpace(); Type *CharTy = Type::getInt8PtrTy(Basis.Ins->getContext(), AS); Reduced = Builder.CreateBitCast(Basis.Ins, CharTy); - if (InBounds) - Reduced = - Builder.CreateInBoundsGEP(Builder.getInt8Ty(), Reduced, Bump); - else - Reduced = Builder.CreateGEP(Builder.getInt8Ty(), Reduced, Bump); + Reduced = + Builder.CreateGEP(Builder.getInt8Ty(), Reduced, Bump, "", InBounds); Reduced = Builder.CreateBitCast(Reduced, C.Ins->getType()); } else { // C = gep Basis, Bump // Canonicalize bump to pointer size. Bump = Builder.CreateSExtOrTrunc(Bump, IntPtrTy); - if (InBounds) - Reduced = Builder.CreateInBoundsGEP( - cast(Basis.Ins)->getResultElementType(), - Basis.Ins, Bump); - else - Reduced = Builder.CreateGEP( - cast(Basis.Ins)->getResultElementType(), - Basis.Ins, Bump); + Reduced = Builder.CreateGEP( + cast(Basis.Ins)->getResultElementType(), + Basis.Ins, Bump, "", InBounds); } break; } diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp index 5869d8aefa77..5b9835a94a6b 100644 --- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp +++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp @@ -9469,11 +9469,8 @@ void VPWidenGEPRecipe::execute(VPTransformState &State) { // Create the new GEP. Note that this GEP may be a scalar if VF == 1, // but it should be a vector, otherwise. - auto *NewGEP = IsInBounds - ? State.Builder.CreateInBoundsGEP( - GEP->getSourceElementType(), Ptr, Indices) - : State.Builder.CreateGEP(GEP->getSourceElementType(), - Ptr, Indices); + auto *NewGEP = State.Builder.CreateGEP(GEP->getSourceElementType(), Ptr, + Indices, "", IsInBounds); assert((State.VF.isScalar() || NewGEP->getType()->isVectorTy()) && "NewGEP is not a pointer vector"); State.set(this, NewGEP, Part); diff --git a/llvm/test/Transforms/InstCombine/gep-combine-loop-invariant.ll b/llvm/test/Transforms/InstCombine/gep-combine-loop-invariant.ll index f9aac12cfb1f..848ce8cb64b2 100644 --- a/llvm/test/Transforms/InstCombine/gep-combine-loop-invariant.ll +++ b/llvm/test/Transforms/InstCombine/gep-combine-loop-invariant.ll @@ -195,7 +195,7 @@ define void @PR51485(<2 x i64> %v) { ; CHECK-NEXT: br label [[LOOP:%.*]] ; CHECK: loop: ; CHECK-NEXT: [[SL1:%.*]] = shl nuw nsw <2 x i64> [[V:%.*]], -; CHECK-NEXT: [[E6:%.*]] = getelementptr inbounds i8, i8* getelementptr (i8, i8* bitcast (void (<2 x i64>)* @PR51485 to i8*), i64 80), <2 x i64> [[SL1]] +; CHECK-NEXT: [[E6:%.*]] = getelementptr inbounds i8, i8* getelementptr inbounds (i8, i8* bitcast (void (<2 x i64>)* @PR51485 to i8*), i64 80), <2 x i64> [[SL1]] ; CHECK-NEXT: call void @blackhole(<2 x i8*> [[E6]]) ; CHECK-NEXT: br label [[LOOP]] ;