<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 23 May 2014 09:42, Robert Lougher <span dir="ltr"><<a href="mailto:rob.lougher@gmail.com" target="_blank">rob.lougher@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi Nick,<br>
<br>
Thanks for replying. Bug filed: <a href="http://llvm.org/bugs/show_bug.cgi?id=19842" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=19842</a></blockquote><div><br></div><div>Thank you!</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Strangely enough, my first conclusion was that %p was being marked<br>
readnone incorrectly as it wasn't handling the copy via @get_addr.<br></blockquote><div><br></div><div>Sorry -- saying %p alone is ambiguous because there's the %p parameter of @<span style="font-family:arial,sans-serif;font-size:13px">get_pntr and the %p parameter of @store. It is correct to mark %p readnone in @get_pntr. From function entrance to exit, it does not write to the pointer passed in. It is incorrect to mark %p readnone in @store, as it receives a pointer then writes to it. Supposing the @get_pntr loaded a pointer from some global variable then wrote through it, then @get_pntr could still mark its %p as being readnone, since the write was not through %p particularly, it just happens to be through a pointer that aliased it.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Somebody (not here) then pointed out the readnone definition in the<br>
LangRef (specifically the "even though it may read or write<br>
<div class="">the memory that the pointer points to if accessed through other<br>
</div>pointers"). I decided the copy fell within this definition and<br>
changed my mind and decided GVN was at fault. How should the<br>
definition be interpreted?<br></blockquote><div><br></div><div>Consider:</div><div><br></div><div>define void @example(i32* %p, i32* %q) {</div><div> store i32 10, i32* %q</div><div> %A = icmp ult i32* %p, %q</div><div>
call void @use(i1 %A) readnone</div><div> ret void</div><div>}</div><div><br></div><div>@example does not modify the memory pointed to by %p, unless it does it through a different pointer, hence it may mark that parameter readnone. A caller may choose to pass in %p == %q in which case the memory pointed-to may change, but @example still lives up to its contract.</div>
<div><br></div><div>The real-world implications of this are that callers may use this information to optimize themselves, combined with other information such as knowing that %p != %q, or better, that %p is a local alloca or freshly malloc'd memory and the pointer %p has not yet escaped (aka. been captured).</div>
<div><br></div><div>If we read the language as permitting the function to make copies of the pointers the this code would be valid:</div><div><br></div><div>define void @writes_over_everything(i32* readnone %p, i32* readnone %q) {</div>
<div> %P = bitcast i32* %p to i32*</div><div> %Q = bitcast i32* %p to i32*</div><div> store i32 10, i32* %P</div><div><div> store i32 10, i32* %Q</div></div><div> ret void</div><div>}</div><div><br></div><div>and that's not useful for optimization.</div>
<div><br></div><div>Nick</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class=""><div class="h5">
On 23 May 2014 04:22, Nick Lewycky <<a href="mailto:nlewycky@google.com">nlewycky@google.com</a>> wrote:<br>
> Confirmed, this is a bug. This<br>
><br>
> define i32* @get_pntr(i32* %p) nounwind uwtable {<br>
> entry:<br>
> ret i32* %p<br>
> }<br>
> define void @store(i32* %p) noinline nounwind uwtable {<br>
> entry:<br>
> %call = call i32* @get_pntr(i32* %p)<br>
> store i32 10, i32* %call, align 4<br>
> ret void<br>
> }<br>
><br>
> run through opt -functionattrs gets a 'readnone' on @store's %p. That's<br>
> wrong, it clearly stores to it. The bug is due to incorrectly handling the<br>
> copy of the pointer made by @get_pntr, because @get_pntr itself is marked<br>
> 'readnone'.<br>
><br>
> Please file a bug.<br>
><br>
> Nick<br>
><br>
><br>
><br>
> On 22 May 2014 08:15, Robert Lougher <<a href="mailto:rob.lougher@gmail.com">rob.lougher@gmail.com</a>> wrote:<br>
>><br>
>> Hi Philip,<br>
>><br>
>> Thanks for the reply.<br>
>><br>
>> On 22 May 2014 01:31, Philip Reames <<a href="mailto:listmail@philipreames.com">listmail@philipreames.com</a>> wrote:<br>
>> ><br>
>> > On 05/21/2014 02:52 PM, Robert Lougher wrote:<br>
>> >><br>
>> >> On 21 May 2014 21:40, Robert Lougher <<a href="mailto:rob.lougher@gmail.com">rob.lougher@gmail.com</a>> wrote:<br>
>> >>><br>
>> >>> define i32* @get_pntr(i32* readnone %p) {<br>
>> >>> entry:<br>
>> >>> ret i32* %p<br>
>> >>> }<br>
>> >>><br>
>> >>> define void @store(i32* nocapture readnone %p) {<br>
>> >>> entry:<br>
>> >>> store i32 10, i32* %p, align 4, !tbaa !1<br>
>> >>> ret void<br>
>> >>> }<br>
>> >>><br>
>> >> Further to my first post, based on the definition of readnone on an<br>
>> >> argument, this is also incorrect. After get_pntr() has been inlined<br>
>> >> into store(), we are dereferencing %p, but it is still marked<br>
>> >> readnone.<br>
>> >><br>
>> >> So we seem to have a couple of issues. First GVN seems to be making<br>
>> >> incorrect assumptions based on argument attributes, and secondly<br>
>> >> inlining can invalidate existing attributes?<br>
>> ><br>
>> > I'm not clear on the GVN issue, but looking at your example IR, the<br>
>> > readnone<br>
>> > attribute is definitely incorrect after inlining.<br>
>> ><br>
>><br>
>> Yes. I haven't investigated it any further, but the parameter was<br>
>> already readnone before inlining, so it looks like the attributes<br>
>> aren't revisited afterwards. But that is just a guess.<br>
>><br>
>> > A question worth asking here: does this definition of readnone make<br>
>> > sense?<br>
>> > I can see where it came from, but it seems to give very unintuitive<br>
>> > reasoning here.<br>
>> ><br>
>><br>
>> Exactly. That was my point in asking here before I went any further.<br>
>> The definition in the LangRef seems odd. If it could still be<br>
>> accessed by another pointer, I can't see where the information is<br>
>> useful.<br>
>><br>
>> > p.s. Is this with TOT or an earlier version?<br>
>><br>
>> Yes, this was with TOT as of yesterday (r209294).<br>
>><br>
>> Thanks,<br>
>> Rob.<br>
>><br>
>> ><br>
>> > Philip<br>
>> ><br>
>> > _______________________________________________<br>
>> > LLVM Developers mailing list<br>
>> > <a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
>> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
>> _______________________________________________<br>
>> LLVM Developers mailing list<br>
>> <a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
><br>
><br>
</div></div></blockquote></div></div></div>