[llvm] r234126 - [opaque pointer type] The last of the GEP IRBuilder API migrations
David Blaikie
dblaikie at gmail.com
Fri Apr 24 07:47:47 PDT 2015
On Fri, Apr 24, 2015 at 1:10 AM, Paweł Bylica <chfast at gmail.com> wrote:
> Will typed pointer go away completely on IR API level?
That's my plan (if there's some reason for that not to be the case -
we should talk about it). Well, there will probably still be the API
for typed pointers to keep the bitcode parser easy to manage (I
suspect it would be excessively complicated to try to parse legacy
pointer types from the bitcode while removing the composite
PointerType entirely - so I might keep it around, rename it to
LegacyPointerType or similar & try to make sure only the bitcode
parser uses it)
>
> 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
More information about the llvm-commits
mailing list