[llvm] r234126 - [opaque pointer type] The last of the GEP IRBuilder API migrations
Hal Finkel
hfinkel at anl.gov
Mon Apr 6 04:33:16 PDT 2015
----- Original Message -----
> From: "Hal Finkel" <hfinkel at anl.gov>
> To: "David Blaikie" <dblaikie at gmail.com>
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Monday, April 6, 2015 4:13:49 AM
> Subject: Re: [llvm] r234126 - [opaque pointer type] The last of the GEP IRBuilder API migrations
>
> ----- Original Message -----
> > From: "David Blaikie" <dblaikie at gmail.com>
> > To: llvm-commits at cs.uiuc.edu
> > Sent: Sunday, April 5, 2015 5:41:44 PM
> > Subject: [llvm] r234126 - [opaque pointer type] The last of the GEP
> > IRBuilder API migrations
> >
> > Author: dblaikie
> > Date: Sun Apr 5 17:41:44 2015
> > New Revision: 234126
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=234126&view=rev
> > Log:
> > [opaque pointer type] The last of the GEP IRBuilder API migrations
> >
> > There's still lots of callers passing nullptr, of course - some
> > because
> > they'll never be migrated (InstCombines for bitcasts - well they
> > don't
> > make any sense when the pointer type is opaque anyway, for example)
> > and
> > others that will need more engineering to pass Types around.
> >
> > Modified:
> > llvm/trunk/include/llvm/IR/IRBuilder.h
> > llvm/trunk/lib/IR/Core.cpp
> > llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
> >
> > Modified: llvm/trunk/include/llvm/IR/IRBuilder.h
> > URL:
> > http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/IRBuilder.h?rev=234126&r1=234125&r2=234126&view=diff
> > ==============================================================================
> > --- llvm/trunk/include/llvm/IR/IRBuilder.h (original)
> > +++ llvm/trunk/include/llvm/IR/IRBuilder.h Sun Apr 5 17:41:44 2015
> > @@ -1102,16 +1102,20 @@ public:
> > }
> > Value *CreateConstInBoundsGEP2_32(Value *Ptr, unsigned Idx0,
> > unsigned Idx1,
> > const Twine &Name = "") {
> > + return CreateConstInBoundsGEP2_32(nullptr, Ptr, Idx0, Idx1,
> > Name);
> > + }
> > + Value *CreateConstInBoundsGEP2_32(Type *Ty, Value *Ptr, unsigned
> > Idx0, unsigned Idx1,
> > + const Twine &Name = "") {
You can ignore this first question because GEP does not do multi-level-pointer indirection (these need to be array types), not sure what I was thinking.
-Hal
>
> Ty is supposed to be the pointer element type of Ptr, right? How is
> this API supposed to work if we have a pointer to a pointer to
> something? If I have i32**, then the pointer element type is i32*,
> and we get:
>
> getelementptr i32*, i32** %ptr, i64 0, i64 2
>
> but once this transition is complete, we'll have only:
>
> getelementptr pointer, pointer %ptr, i64 0, i64 2
>
> and we won't have information on the size of the pointer element's
> element type, and thus, can't resolve the offset implied by the last
> index. Are we going to simply forbid using GEPs this way? If so, is
> that restriction going to be reflected in the IRBuilder interface,
> or is IRBuilder going to hide that (or make it easier) somehow?
>
> Also, are there any asserts anywhere that trigger if we get these
> types wrong? I'm thinking of something like
>
> assert(!Ty || Ty == Ptr->getPointerElementType())
>
> but maybe these are just further down the call chain?
>
> Thanks again,
> Hal
>
> > Value *Idxs[] = {
> > ConstantInt::get(Type::getInt32Ty(Context), Idx0),
> > ConstantInt::get(Type::getInt32Ty(Context), Idx1)
> > };
> >
> > if (Constant *PC = dyn_cast<Constant>(Ptr))
> > - return Insert(Folder.CreateInBoundsGetElementPtr(nullptr,
> > PC,
> > Idxs),
> > + return Insert(Folder.CreateInBoundsGetElementPtr(Ty, PC,
> > Idxs),
> > Name);
> >
> > - return Insert(GetElementPtrInst::CreateInBounds(nullptr, Ptr,
> > Idxs), Name);
> > + return Insert(GetElementPtrInst::CreateInBounds(Ty, Ptr,
> > Idxs),
> > Name);
> > }
> > Value *CreateConstGEP1_64(Value *Ptr, uint64_t Idx0, const Twine
> > &Name = "") {
> > Value *Idx = ConstantInt::get(Type::getInt64Ty(Context),
> > Idx0);
> > @@ -1156,7 +1160,10 @@ public:
> > return Insert(GetElementPtrInst::CreateInBounds(nullptr, Ptr,
> > Idxs), Name);
> > }
> > Value *CreateStructGEP(Value *Ptr, unsigned Idx, const Twine
> > &Name
> > = "") {
> > - return CreateConstInBoundsGEP2_32(Ptr, 0, Idx, Name);
> > + return CreateStructGEP(nullptr, Ptr, Idx, Name);
> > + }
> > + Value *CreateStructGEP(Type *Ty, Value *Ptr, unsigned Idx, const
> > Twine &Name = "") {
> > + return CreateConstInBoundsGEP2_32(Ty, Ptr, 0, Idx, Name);
> > }
> >
> > /// \brief Same as CreateGlobalString, but return a pointer with
> > "i8*" type
> >
> > Modified: llvm/trunk/lib/IR/Core.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Core.cpp?rev=234126&r1=234125&r2=234126&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/IR/Core.cpp (original)
> > +++ llvm/trunk/lib/IR/Core.cpp Sun Apr 5 17:41:44 2015
> > @@ -2518,7 +2518,7 @@ LLVMValueRef LLVMBuildInBoundsGEP(LLVMBu
> >
> > LLVMValueRef LLVMBuildStructGEP(LLVMBuilderRef B, LLVMValueRef
> > Pointer,
> > unsigned Idx, const char *Name) {
> > - return wrap(unwrap(B)->CreateStructGEP(unwrap(Pointer), Idx,
> > Name));
> > + return wrap(unwrap(B)->CreateStructGEP(nullptr, unwrap(Pointer),
> > Idx, Name));
> > }
> >
> > LLVMValueRef LLVMBuildGlobalString(LLVMBuilderRef B, const char
> > *Str,
> >
> > Modified:
> > llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp?rev=234126&r1=234125&r2=234126&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
> > (original)
> > +++ llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
> > Sun Apr 5 17:41:44 2015
> > @@ -1472,17 +1472,16 @@ void DFSanVisitor::visitCallSite(CallSit
> > Args.push_back(DFSF.getShadow(*i));
> >
> > if (FT->isVarArg()) {
> > - auto LabelVAAlloca =
> > - new AllocaInst(ArrayType::get(DFSF.DFS.ShadowTy,
> > - CS.arg_size() -
> > FT->getNumParams()),
> > - "labelva",
> > DFSF.F->getEntryBlock().begin());
> > + auto *LabelVATy = ArrayType::get(DFSF.DFS.ShadowTy,
> > CS.arg_size() - FT->getNumParams());
> > + auto *LabelVAAlloca = new AllocaInst(LabelVATy,
> > "labelva",
> > +
> > DFSF.F->getEntryBlock().begin());
> >
> > for (unsigned n = 0; i != CS.arg_end(); ++i, ++n) {
> > - auto LabelVAPtr = IRB.CreateStructGEP(LabelVAAlloca,
> > n);
> > + auto LabelVAPtr = IRB.CreateStructGEP(LabelVATy,
> > LabelVAAlloca, n);
> > IRB.CreateStore(DFSF.getShadow(*i), LabelVAPtr);
> > }
> >
> > - Args.push_back(IRB.CreateStructGEP(LabelVAAlloca, 0));
> > + Args.push_back(IRB.CreateStructGEP(LabelVATy,
> > LabelVAAlloca, 0));
> > }
> >
> > if (!FT->getReturnType()->isVoidTy()) {
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list