[llvm] r234126 - [opaque pointer type] The last of the GEP IRBuilder API migrations

PaweĊ‚ Bylica chfast at gmail.com
Fri Apr 24 01:10:36 PDT 2015


Will typed pointer go away completely on IR API level?

On Mon, Apr 6, 2015 at 4:48 PM David Blaikie <dblaikie at gmail.com> wrote:

> 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
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150424/67ab568c/attachment.html>


More information about the llvm-commits mailing list