<div dir="ltr">Will typed pointer go away completely on IR API level? <br></div><br><div class="gmail_quote">On Mon, Apr 6, 2015 at 4:48 PM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Apr 6, 2015 at 4:33 AM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>----- Original Message -----<br>
> From: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>><br>
> To: "David Blaikie" <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>><br>
> Cc: <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> Sent: Monday, April 6, 2015 4:13:49 AM<br>
> Subject: Re: [llvm] r234126 - [opaque pointer type] The last of the GEP IRBuilder API migrations<br>
><br>
> ----- Original Message -----<br>
> > From: "David Blaikie" <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>><br>
> > To: <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> > Sent: Sunday, April 5, 2015 5:41:44 PM<br>
> > Subject: [llvm] r234126 - [opaque pointer type] The last of the GEP<br>
> > IRBuilder API migrations<br>
> ><br>
> > Author: dblaikie<br>
> > Date: Sun Apr 5 17:41:44 2015<br>
> > New Revision: 234126<br>
> ><br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=234126&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=234126&view=rev</a><br>
> > Log:<br>
> > [opaque pointer type] The last of the GEP IRBuilder API migrations<br>
> ><br>
> > There's still lots of callers passing nullptr, of course - some<br>
> > because<br>
> > they'll never be migrated (InstCombines for bitcasts - well they<br>
> > don't<br>
> > make any sense when the pointer type is opaque anyway, for example)<br>
> > and<br>
> > others that will need more engineering to pass Types around.<br>
> ><br>
> > Modified:<br>
> > llvm/trunk/include/llvm/IR/IRBuilder.h<br>
> > llvm/trunk/lib/IR/Core.cpp<br>
> > llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp<br>
> ><br>
> > Modified: llvm/trunk/include/llvm/IR/IRBuilder.h<br>
> > URL:<br>
> > <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/IRBuilder.h?rev=234126&r1=234125&r2=234126&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/IRBuilder.h?rev=234126&r1=234125&r2=234126&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/include/llvm/IR/IRBuilder.h (original)<br>
> > +++ llvm/trunk/include/llvm/IR/IRBuilder.h Sun Apr 5 17:41:44 2015<br>
> > @@ -1102,16 +1102,20 @@ public:<br>
> > }<br>
> > Value *CreateConstInBoundsGEP2_32(Value *Ptr, unsigned Idx0,<br>
> > unsigned Idx1,<br>
> > const Twine &Name = "") {<br>
> > + return CreateConstInBoundsGEP2_32(nullptr, Ptr, Idx0, Idx1,<br>
> > Name);<br>
> > + }<br>
> > + Value *CreateConstInBoundsGEP2_32(Type *Ty, Value *Ptr, unsigned<br>
> > Idx0, unsigned Idx1,<br>
> > + const Twine &Name = "") {<br>
<br>
<br>
</div></div>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.<br>
<span><font color="#888888"><br>
-Hal<br>
</font></span><div><div><br>
><br>
> Ty is supposed to be the pointer element type of Ptr, right? How is<br>
> this API supposed to work if we have a pointer to a pointer to<br>
> something? If I have i32**, then the pointer element type is i32*,<br>
> and we get:<br>
><br>
> getelementptr i32*, i32** %ptr, i64 0, i64 2<br>
><br>
> but once this transition is complete, we'll have only:<br>
><br>
> getelementptr pointer, pointer %ptr, i64 0, i64 2<br>
><br>
> and we won't have information on the size of the pointer element's<br>
> element type, and thus, can't resolve the offset implied by the last<br>
> index. Are we going to simply forbid using GEPs this way? If so, is<br>
> that restriction going to be reflected in the IRBuilder interface,<br>
> or is IRBuilder going to hide that (or make it easier) somehow?<br>
><br>
> Also, are there any asserts anywhere that trigger if we get these<br>
> types wrong? I'm thinking of something like<br>
><br>
> assert(!Ty || Ty == Ptr->getPointerElementType())<br>
><br>
> but maybe these are just further down the call chain?<br></div></div></blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br>Yep, this assertion is down in GetElementPtrInst::Create (and in Constants::getGetElementPtr in one or two places, if I recall correctly).<br><br>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).<br><br>It's just a lot of work and I'm taking it in very small steps... <br><br>Hope that helps - let me know if that doesn't make sense/hasn't addressed the questions you were still asking,<br><br>- Dave<br> </div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>
><br>
> Thanks again,<br>
> Hal<br>
><br>
> > Value *Idxs[] = {<br>
> > ConstantInt::get(Type::getInt32Ty(Context), Idx0),<br>
> > ConstantInt::get(Type::getInt32Ty(Context), Idx1)<br>
> > };<br>
> ><br>
> > if (Constant *PC = dyn_cast<Constant>(Ptr))<br>
> > - return Insert(Folder.CreateInBoundsGetElementPtr(nullptr,<br>
> > PC,<br>
> > Idxs),<br>
> > + return Insert(Folder.CreateInBoundsGetElementPtr(Ty, PC,<br>
> > Idxs),<br>
> > Name);<br>
> ><br>
> > - return Insert(GetElementPtrInst::CreateInBounds(nullptr, Ptr,<br>
> > Idxs), Name);<br>
> > + return Insert(GetElementPtrInst::CreateInBounds(Ty, Ptr,<br>
> > Idxs),<br>
> > Name);<br>
> > }<br>
> > Value *CreateConstGEP1_64(Value *Ptr, uint64_t Idx0, const Twine<br>
> > &Name = "") {<br>
> > Value *Idx = ConstantInt::get(Type::getInt64Ty(Context),<br>
> > Idx0);<br>
> > @@ -1156,7 +1160,10 @@ public:<br>
> > return Insert(GetElementPtrInst::CreateInBounds(nullptr, Ptr,<br>
> > Idxs), Name);<br>
> > }<br>
> > Value *CreateStructGEP(Value *Ptr, unsigned Idx, const Twine<br>
> > &Name<br>
> > = "") {<br>
> > - return CreateConstInBoundsGEP2_32(Ptr, 0, Idx, Name);<br>
> > + return CreateStructGEP(nullptr, Ptr, Idx, Name);<br>
> > + }<br>
> > + Value *CreateStructGEP(Type *Ty, Value *Ptr, unsigned Idx, const<br>
> > Twine &Name = "") {<br>
> > + return CreateConstInBoundsGEP2_32(Ty, Ptr, 0, Idx, Name);<br>
> > }<br>
> ><br>
> > /// \brief Same as CreateGlobalString, but return a pointer with<br>
> > "i8*" type<br>
> ><br>
> > Modified: llvm/trunk/lib/IR/Core.cpp<br>
> > URL:<br>
> > <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Core.cpp?rev=234126&r1=234125&r2=234126&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Core.cpp?rev=234126&r1=234125&r2=234126&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/lib/IR/Core.cpp (original)<br>
> > +++ llvm/trunk/lib/IR/Core.cpp Sun Apr 5 17:41:44 2015<br>
> > @@ -2518,7 +2518,7 @@ LLVMValueRef LLVMBuildInBoundsGEP(LLVMBu<br>
> ><br>
> > LLVMValueRef LLVMBuildStructGEP(LLVMBuilderRef B, LLVMValueRef<br>
> > Pointer,<br>
> > unsigned Idx, const char *Name) {<br>
> > - return wrap(unwrap(B)->CreateStructGEP(unwrap(Pointer), Idx,<br>
> > Name));<br>
> > + return wrap(unwrap(B)->CreateStructGEP(nullptr, unwrap(Pointer),<br>
> > Idx, Name));<br>
> > }<br>
> ><br>
> > LLVMValueRef LLVMBuildGlobalString(LLVMBuilderRef B, const char<br>
> > *Str,<br>
> ><br>
> > Modified:<br>
> > llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp<br>
> > URL:<br>
> > <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp?rev=234126&r1=234125&r2=234126&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp?rev=234126&r1=234125&r2=234126&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp<br>
> > (original)<br>
> > +++ llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp<br>
> > Sun Apr 5 17:41:44 2015<br>
> > @@ -1472,17 +1472,16 @@ void DFSanVisitor::visitCallSite(CallSit<br>
> > Args.push_back(DFSF.getShadow(*i));<br>
> ><br>
> > if (FT->isVarArg()) {<br>
> > - auto LabelVAAlloca =<br>
> > - new AllocaInst(ArrayType::get(DFSF.DFS.ShadowTy,<br>
> > - CS.arg_size() -<br>
> > FT->getNumParams()),<br>
> > - "labelva",<br>
> > DFSF.F->getEntryBlock().begin());<br>
> > + auto *LabelVATy = ArrayType::get(DFSF.DFS.ShadowTy,<br>
> > CS.arg_size() - FT->getNumParams());<br>
> > + auto *LabelVAAlloca = new AllocaInst(LabelVATy,<br>
> > "labelva",<br>
> > +<br>
> > DFSF.F->getEntryBlock().begin());<br>
> ><br>
> > for (unsigned n = 0; i != CS.arg_end(); ++i, ++n) {<br>
> > - auto LabelVAPtr = IRB.CreateStructGEP(LabelVAAlloca,<br>
> > n);<br>
> > + auto LabelVAPtr = IRB.CreateStructGEP(LabelVATy,<br>
> > LabelVAAlloca, n);<br>
> > IRB.CreateStore(DFSF.getShadow(*i), LabelVAPtr);<br>
> > }<br>
> ><br>
> > - Args.push_back(IRB.CreateStructGEP(LabelVAAlloca, 0));<br>
> > + Args.push_back(IRB.CreateStructGEP(LabelVATy,<br>
> > LabelVAAlloca, 0));<br>
> > }<br>
> ><br>
> > if (!FT->getReturnType()->isVoidTy()) {<br>
> ><br>
> ><br>
> > _______________________________________________<br>
> > llvm-commits mailing list<br>
> > <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
> ><br>
><br>
> --<br>
> Hal Finkel<br>
> Assistant Computational Scientist<br>
> Leadership Computing Facility<br>
> Argonne National Laboratory<br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
<br>
--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</div></div></blockquote></div></div></div>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>