<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 5, 2016 at 12:40 PM, Sanjoy Das <span dir="ltr"><<a href="mailto:sanjoy@playingwithpointers.com" target="_blank">sanjoy@playingwithpointers.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Daniel,<span class=""><br>
<br>
Daniel Berlin wrote:<br>
> I really want to say yes, but ....<br>
><br>
> 1. Your argument is identical for the other intrisics right above it.<br>
> None of them touch memory in practice.<br>
> 2. MDA is deliberately returning it as a Dep so that things know where<br>
> the invariants start and end (again, same with lifetime markers).<br>
> I realize this is not ideal (and in fact, sucks), but things depend on it.<br>
><br>
> 3. Without this, you will enable hoisting that is not allowed, if things<br>
> use MDA to perform hoisting.<br>
><br>
> You cannot move the invariant start and end calls, and MDA saying they<br>
> are read-only will enable them to be moved.<br>
<br></span>
Are you saying that they'll get speculated out of control flow?  </blockquote><div><br></div><div>Yes, i'm saying the  mechanism currently used to keep them from moving is to say they read/write memory.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">That<br>
does not seem safe -- how does LLVM know that they won't segfault?<span class=""><br>
</span></blockquote><div><br></div><div><br></div><div>I'm not sure i understand.</div><div>These are invariant start and end markers, they generate no real code.</div><div>If you move them, the problem is that the definition of invariant.start says:<br>"<span style="color:rgb(0,0,0);font-family:"Lucida Grande","Lucida Sans Unicode",Geneva,Verdana,sans-serif;font-size:14px">This intrinsic indicates that until an</span><span style="color:rgb(0,0,0);font-family:"Lucida Grande","Lucida Sans Unicode",Geneva,Verdana,sans-serif;font-size:14px"> </span><code class="" style="color:rgb(0,0,0);font-family:Consolas,"Deja Vu Sans Mono","Bitstream Vera Sans Mono",monospace;font-size:0.95em"><span class="">llvm.invariant.end</span></code><span style="color:rgb(0,0,0);font-family:"Lucida Grande","Lucida Sans Unicode",Geneva,Verdana,sans-serif;font-size:14px"> </span><span style="color:rgb(0,0,0);font-family:"Lucida Grande","Lucida Sans Unicode",Geneva,Verdana,sans-serif;font-size:14px">that uses the return value, the referenced memory location is constant and unchanging.</span></div><div class="" id="llvm-invariant-end-intrinsic" style="color:rgb(0,0,0);font-family:"Lucida Grande","Lucida Sans Unicode",Geneva,Verdana,sans-serif;font-size:14px"></div><div>"</div><div><br></div><div>So if you hoist/sink start or end , you change the bounds of where the memory is unchanging.</div><div><br></div><div>You could fix this, of course, by having the invariant.start and invariant.end calls return pointers, and all things that are invariant use those pointers (or pointers derived from those pointers).  IE express the dependence in SSA directly.</div><div><br></div><div>Then you can move the start/end calls wherever they are valid (IE wherever they are dominated by their operands) and it will not change the semantic effect.</div><div><br></div><div>But that's not the current design.</div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
> We last tried this by saying they did not touch memory (which is<br>
> correct), and peter had to revert the set of changes that gave them "The<br>
> correct answer" because it broke on real code.<br>
<br></span>
Do you happen to remember any relevant revision numbers / zilla bugs<br>
for this?<span class=""><font color="#888888"><br>
<br></font></span></blockquote><div>The revert was: "<span style="font-size:inherit;font-weight:inherit">[llvm] r270823 - MemorySSA: Revert r269678 and r268068; replace with special casing in MemorySSA.</span></div><div>"</div><div><br></div><div>This is when we tried this with assume </div><div>Look for emails from pcc around the time you see "[PATCH] D20653: LICM: Do not sink or hoist assume intrinsic calls."</div><div><br>This was a set of followups from peter to try to starting fixing places that tried to hoist them anyway, and it turns out ... it's not a pretty path.</div><div>;)<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class=""><font color="#888888">
-- Sanjoy<br>
</font></span></blockquote></div><br></div></div>