[llvm] r234126 - [opaque pointer type] The last of the GEP IRBuilder API migrations
David Blaikie
dblaikie at gmail.com
Mon Apr 6 07:44:54 PDT 2015
On Mon, Apr 6, 2015 at 4:33 AM, Hal Finkel <hfinkel at anl.gov> wrote:
> ----- 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?
>
Yep, this assertion is down in GetElementPtrInst::Create (and in
Constants::getGetElementPtr in one or two places, if I recall correctly).
For now they allow null types - eventually that will go away & the type
will have to be passed. The same is going to be true of Load instructions
(if you want to load a pointer (so you pass pointer to pointer) then you
can do that, but there won't be any extra pointer indirection you can
specify (eg: you can't say "load a value that will point to an int" it'll
just be an opaque pointer, so it'll be "load ptr, ptr %x" (load a pointer
from a pointer)) and various other things (we still have to settle on byval
for instance).
It's just a lot of work and I'm taking it in very small steps...
Hope that helps - let me know if that doesn't make sense/hasn't addressed
the questions you were still asking,
- Dave
> >
> > 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150406/b8efb44d/attachment.html>
More information about the llvm-commits
mailing list