From 741c8f81e469930b7c72a59c82adae12c933b1e2 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Sat, 14 Mar 2015 01:53:18 +0000 Subject: [PATCH] [opaque pointer type] Start migrating GEP creation to explicitly specify the pointee type I'm just going to migrate these in a pretty ad-hoc & incremental way - providing the backwards compatible API for now, then locally removing it, fixing a few callers, adding it back in and commiting those callers. Rinse, repeat. The assertions should ensure that if I get this wrong we'll find out about it and not just have one giant patch to revert, recommit, revert, recommit, etc. llvm-svn: 232240 --- llvm/include/llvm/IR/Instructions.h | 79 +++++++++++++++------- llvm/lib/IR/Constants.cpp | 14 ++-- llvm/lib/Transforms/IPO/GlobalOpt.cpp | 10 +-- llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp | 3 +- .../Scalar/SeparateConstOffsetFromGEP.cpp | 10 +-- llvm/lib/Transforms/Utils/CodeExtractor.cpp | 30 ++++---- 6 files changed, 88 insertions(+), 58 deletions(-) diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h index 1ae1757..2e84d95 100644 --- a/llvm/include/llvm/IR/Instructions.h +++ b/llvm/include/llvm/IR/Instructions.h @@ -794,28 +794,41 @@ class GetElementPtrInst : public Instruction { /// list of indices. The first ctor can optionally insert before an existing /// instruction, the second appends the new instruction to the specified /// BasicBlock. - inline GetElementPtrInst(Value *Ptr, ArrayRef IdxList, - unsigned Values, const Twine &NameStr, - Instruction *InsertBefore); - inline GetElementPtrInst(Value *Ptr, ArrayRef IdxList, - unsigned Values, const Twine &NameStr, - BasicBlock *InsertAtEnd); + inline GetElementPtrInst(Type *PointeeType, Value *Ptr, + ArrayRef IdxList, unsigned Values, + const Twine &NameStr, Instruction *InsertBefore); + inline GetElementPtrInst(Type *PointeeType, Value *Ptr, + ArrayRef IdxList, unsigned Values, + const Twine &NameStr, BasicBlock *InsertAtEnd); + protected: GetElementPtrInst *clone_impl() const override; public: static GetElementPtrInst *Create(Value *Ptr, ArrayRef IdxList, const Twine &NameStr = "", Instruction *InsertBefore = nullptr) { + return Create(nullptr, Ptr, IdxList, NameStr, InsertBefore); + } + static GetElementPtrInst *Create(Type *PointeeType, Value *Ptr, + ArrayRef IdxList, + const Twine &NameStr = "", + Instruction *InsertBefore = nullptr) { unsigned Values = 1 + unsigned(IdxList.size()); - return new(Values) - GetElementPtrInst(Ptr, IdxList, Values, NameStr, InsertBefore); + return new (Values) GetElementPtrInst(PointeeType, Ptr, IdxList, Values, + NameStr, InsertBefore); } static GetElementPtrInst *Create(Value *Ptr, ArrayRef IdxList, const Twine &NameStr, BasicBlock *InsertAtEnd) { + return Create(nullptr, Ptr, NameStr, InsertAtEnd); + } + static GetElementPtrInst *Create(Type *PointeeType, Value *Ptr, + ArrayRef IdxList, + const Twine &NameStr, + BasicBlock *InsertAtEnd) { unsigned Values = 1 + unsigned(IdxList.size()); - return new(Values) - GetElementPtrInst(Ptr, IdxList, Values, NameStr, InsertAtEnd); + return new (Values) GetElementPtrInst(PointeeType, Ptr, IdxList, Values, + NameStr, InsertAtEnd); } /// Create an "inbounds" getelementptr. See the documentation for the @@ -824,7 +837,14 @@ public: ArrayRef IdxList, const Twine &NameStr = "", Instruction *InsertBefore = nullptr){ - GetElementPtrInst *GEP = Create(Ptr, IdxList, NameStr, InsertBefore); + return CreateInBounds(nullptr, Ptr, IdxList, NameStr, InsertBefore); + } + static GetElementPtrInst * + CreateInBounds(Type *PointeeType, Value *Ptr, ArrayRef IdxList, + const Twine &NameStr = "", + Instruction *InsertBefore = nullptr) { + GetElementPtrInst *GEP = + Create(PointeeType, Ptr, IdxList, NameStr, InsertBefore); GEP->setIsInBounds(true); return GEP; } @@ -832,7 +852,14 @@ public: ArrayRef IdxList, const Twine &NameStr, BasicBlock *InsertAtEnd) { - GetElementPtrInst *GEP = Create(Ptr, IdxList, NameStr, InsertAtEnd); + return CreateInBounds(nullptr, Ptr, IdxList, NameStr, InsertAtEnd); + } + static GetElementPtrInst *CreateInBounds(Type *PointeeType, Value *Ptr, + ArrayRef IdxList, + const Twine &NameStr, + BasicBlock *InsertAtEnd) { + GetElementPtrInst *GEP = + Create(PointeeType, Ptr, IdxList, NameStr, InsertAtEnd); GEP->setIsInBounds(true); return GEP; } @@ -850,6 +877,8 @@ public: ->getElementType(); } + Type *getResultElementType() const { return getType()->getElementType(); } + /// \brief Returns the address space of this instruction's pointer type. unsigned getAddressSpace() const { // Note that this is always the same as the pointer operand's address space @@ -958,27 +987,25 @@ struct OperandTraits : public VariadicOperandTraits { }; -GetElementPtrInst::GetElementPtrInst(Value *Ptr, - ArrayRef IdxList, - unsigned Values, +GetElementPtrInst::GetElementPtrInst(Type *PointeeType, Value *Ptr, + ArrayRef IdxList, unsigned Values, const Twine &NameStr, Instruction *InsertBefore) - : Instruction(getGEPReturnType(Ptr, IdxList), - GetElementPtr, - OperandTraits::op_end(this) - Values, - Values, InsertBefore) { + : Instruction(getGEPReturnType(Ptr, IdxList), GetElementPtr, + OperandTraits::op_end(this) - Values, + Values, InsertBefore) { init(Ptr, IdxList, NameStr); + assert(!PointeeType || PointeeType == getSourceElementType()); } -GetElementPtrInst::GetElementPtrInst(Value *Ptr, - ArrayRef IdxList, - unsigned Values, +GetElementPtrInst::GetElementPtrInst(Type *PointeeType, Value *Ptr, + ArrayRef IdxList, unsigned Values, const Twine &NameStr, BasicBlock *InsertAtEnd) - : Instruction(getGEPReturnType(Ptr, IdxList), - GetElementPtr, - OperandTraits::op_end(this) - Values, - Values, InsertAtEnd) { + : Instruction(getGEPReturnType(Ptr, IdxList), GetElementPtr, + OperandTraits::op_end(this) - Values, + Values, InsertAtEnd) { init(Ptr, IdxList, NameStr); + assert(!PointeeType || PointeeType == getSourceElementType()); } diff --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp index f5d901c..e51a396 100644 --- a/llvm/lib/IR/Constants.cpp +++ b/llvm/lib/IR/Constants.cpp @@ -3001,12 +3001,14 @@ Instruction *ConstantExpr::getAsInstruction() { case Instruction::ShuffleVector: return new ShuffleVectorInst(Ops[0], Ops[1], Ops[2]); - case Instruction::GetElementPtr: - if (cast(this)->isInBounds()) - return GetElementPtrInst::CreateInBounds(Ops[0], Ops.slice(1)); - else - return GetElementPtrInst::Create(Ops[0], Ops.slice(1)); - + case Instruction::GetElementPtr: { + const auto *GO = cast(this); + if (GO->isInBounds()) + return GetElementPtrInst::CreateInBounds(GO->getSourceElementType(), + Ops[0], Ops.slice(1)); + return GetElementPtrInst::Create(GO->getSourceElementType(), Ops[0], + Ops.slice(1)); + } case Instruction::ICmp: case Instruction::FCmp: return CmpInst::Create((Instruction::OtherOps)getOpcode(), diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp index c62eeec..26dc991 100644 --- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp +++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp @@ -579,8 +579,9 @@ static GlobalVariable *SRAGlobal(GlobalVariable *GV, const DataLayout &DL) { Idxs.push_back(NullInt); for (unsigned i = 3, e = GEPI->getNumOperands(); i != e; ++i) Idxs.push_back(GEPI->getOperand(i)); - NewPtr = GetElementPtrInst::Create(NewPtr, Idxs, - GEPI->getName()+"."+Twine(Val),GEPI); + NewPtr = GetElementPtrInst::Create( + NewPtr->getType()->getPointerElementType(), NewPtr, Idxs, + GEPI->getName() + "." + Twine(Val), GEPI); } } GEP->replaceAllUsesWith(NewPtr); @@ -1164,7 +1165,8 @@ static Value *GetHeapSROAValue(Value *V, unsigned FieldNo, InsertedScalarizedValues, PHIsToRewrite), LI->getName()+".f"+Twine(FieldNo), LI); - } else if (PHINode *PN = dyn_cast(V)) { + } else { + PHINode *PN = cast(V); // PN's type is pointer to struct. Make a new PHI of pointer to struct // field. @@ -1178,8 +1180,6 @@ static Value *GetHeapSROAValue(Value *V, unsigned FieldNo, PN->getName()+".f"+Twine(FieldNo), PN); Result = NewPN; PHIsToRewrite.push_back(std::make_pair(PN, FieldNo)); - } else { - llvm_unreachable("Unknown usable value"); } return FieldVals[FieldNo] = Result; diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp index 76f8927..ca2caed 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp @@ -230,7 +230,8 @@ Instruction *InstCombiner::FoldPHIArgGEPIntoPHI(PHINode &PN) { Value *Base = FixedOperands[0]; GetElementPtrInst *NewGEP = - GetElementPtrInst::Create(Base, makeArrayRef(FixedOperands).slice(1)); + GetElementPtrInst::Create(FirstInst->getSourceElementType(), Base, + makeArrayRef(FixedOperands).slice(1)); if (AllInBounds) NewGEP->setIsInBounds(); NewGEP->setDebugLoc(FirstInst->getDebugLoc()); return NewGEP; diff --git a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp index 3d5f6c9..1a04d74 100644 --- a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp +++ b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp @@ -952,8 +952,9 @@ bool SeparateConstOffsetFromGEP::splitGEP(GetElementPtrInst *GEP) { // Very likely. As long as %gep is natually aligned, the byte offset we // extracted should be a multiple of sizeof(*%gep). int64_t Index = AccumulativeByteOffset / ElementTypeSizeOfGEP; - NewGEP = GetElementPtrInst::Create( - NewGEP, ConstantInt::get(IntPtrTy, Index, true), GEP->getName(), GEP); + NewGEP = GetElementPtrInst::Create(GEP->getResultElementType(), NewGEP, + ConstantInt::get(IntPtrTy, Index, true), + GEP->getName(), GEP); } else { // Unlikely but possible. For example, // #pragma pack(1) @@ -973,8 +974,9 @@ bool SeparateConstOffsetFromGEP::splitGEP(GetElementPtrInst *GEP) { GEP->getPointerAddressSpace()); NewGEP = new BitCastInst(NewGEP, I8PtrTy, "", GEP); NewGEP = GetElementPtrInst::Create( - NewGEP, ConstantInt::get(IntPtrTy, AccumulativeByteOffset, true), - "uglygep", GEP); + Type::getInt8Ty(GEP->getContext()), NewGEP, + ConstantInt::get(IntPtrTy, AccumulativeByteOffset, true), "uglygep", + GEP); if (GEP->getType() != I8PtrTy) NewGEP = new BitCastInst(NewGEP, GEP->getType(), GEP->getName(), GEP); } diff --git a/llvm/lib/Transforms/Utils/CodeExtractor.cpp b/llvm/lib/Transforms/Utils/CodeExtractor.cpp index e70a7d6..ab89b41 100644 --- a/llvm/lib/Transforms/Utils/CodeExtractor.cpp +++ b/llvm/lib/Transforms/Utils/CodeExtractor.cpp @@ -332,11 +332,11 @@ Function *CodeExtractor::constructFunction(const ValueSet &inputs, DEBUG(dbgs() << **i << ", "); DEBUG(dbgs() << ")\n"); + StructType *StructTy; if (AggregateArgs && (inputs.size() + outputs.size() > 0)) { - PointerType *StructPtr = - PointerType::getUnqual(StructType::get(M->getContext(), paramTy)); + StructTy = StructType::get(M->getContext(), paramTy); paramTy.clear(); - paramTy.push_back(StructPtr); + paramTy.push_back(PointerType::getUnqual(StructTy)); } FunctionType *funcType = FunctionType::get(RetTy, paramTy, false); @@ -364,8 +364,8 @@ Function *CodeExtractor::constructFunction(const ValueSet &inputs, Idx[0] = Constant::getNullValue(Type::getInt32Ty(header->getContext())); Idx[1] = ConstantInt::get(Type::getInt32Ty(header->getContext()), i); TerminatorInst *TI = newFunction->begin()->getTerminator(); - GetElementPtrInst *GEP = - GetElementPtrInst::Create(AI, Idx, "gep_" + inputs[i]->getName(), TI); + GetElementPtrInst *GEP = GetElementPtrInst::Create( + StructTy, AI, Idx, "gep_" + inputs[i]->getName(), TI); RewriteVal = new LoadInst(GEP, "loadgep_" + inputs[i]->getName(), TI); } else RewriteVal = AI++; @@ -447,6 +447,7 @@ emitCallAndSwitchStatement(Function *newFunction, BasicBlock *codeReplacer, } } + StructType *StructArgTy = nullptr; AllocaInst *Struct = nullptr; if (AggregateArgs && (inputs.size() + outputs.size() > 0)) { std::vector ArgTypes; @@ -455,7 +456,7 @@ emitCallAndSwitchStatement(Function *newFunction, BasicBlock *codeReplacer, ArgTypes.push_back((*v)->getType()); // Allocate a struct at the beginning of this function - Type *StructArgTy = StructType::get(newFunction->getContext(), ArgTypes); + StructArgTy = StructType::get(newFunction->getContext(), ArgTypes); Struct = new AllocaInst(StructArgTy, nullptr, "structArg", codeReplacer->getParent()->begin()->begin()); @@ -465,9 +466,8 @@ emitCallAndSwitchStatement(Function *newFunction, BasicBlock *codeReplacer, Value *Idx[2]; Idx[0] = Constant::getNullValue(Type::getInt32Ty(Context)); Idx[1] = ConstantInt::get(Type::getInt32Ty(Context), i); - GetElementPtrInst *GEP = - GetElementPtrInst::Create(Struct, Idx, - "gep_" + StructValues[i]->getName()); + GetElementPtrInst *GEP = GetElementPtrInst::Create( + StructArgTy, Struct, Idx, "gep_" + StructValues[i]->getName()); codeReplacer->getInstList().push_back(GEP); StoreInst *SI = new StoreInst(StructValues[i], GEP); codeReplacer->getInstList().push_back(SI); @@ -491,9 +491,8 @@ emitCallAndSwitchStatement(Function *newFunction, BasicBlock *codeReplacer, Value *Idx[2]; Idx[0] = Constant::getNullValue(Type::getInt32Ty(Context)); Idx[1] = ConstantInt::get(Type::getInt32Ty(Context), FirstOut + i); - GetElementPtrInst *GEP - = GetElementPtrInst::Create(Struct, Idx, - "gep_reload_" + outputs[i]->getName()); + GetElementPtrInst *GEP = GetElementPtrInst::Create( + StructArgTy, Struct, Idx, "gep_reload_" + outputs[i]->getName()); codeReplacer->getInstList().push_back(GEP); Output = GEP; } else { @@ -606,10 +605,9 @@ emitCallAndSwitchStatement(Function *newFunction, BasicBlock *codeReplacer, Idx[0] = Constant::getNullValue(Type::getInt32Ty(Context)); Idx[1] = ConstantInt::get(Type::getInt32Ty(Context), FirstOut+out); - GetElementPtrInst *GEP = - GetElementPtrInst::Create(OAI, Idx, - "gep_" + outputs[out]->getName(), - NTRet); + GetElementPtrInst *GEP = GetElementPtrInst::Create( + StructArgTy, OAI, Idx, "gep_" + outputs[out]->getName(), + NTRet); new StoreInst(outputs[out], GEP, NTRet); } else { new StoreInst(outputs[out], OAI, NTRet); -- 2.7.4