<div dir="ltr">I really want to say yes, but ....<div><br></div><div>1. Your argument is identical for the other intrisics right above it. None of them touch memory in practice.<div>2. MDA is deliberately returning it as a Dep so that things know where the invariants start and end (again, same with lifetime markers).</div><div>I realize this is not ideal (and in fact, sucks), but things depend on it.<br></div><div><br></div><div>3. Without this, you will enable hoisting that is not allowed, if things use MDA to perform hoisting.</div></div><div><br></div><div>You cannot move the invariant start and end calls, and MDA saying they are read-only will enable them to be moved.</div><div>We last tried this by saying they did not touch memory (which is correct), and peter had to revert the set of changes that gave them "The correct answer" because it broke on real code.</div><div><br></div><div>I don't think we are going to end up with different situation here.</div><div><br></div><div>Instead, once we have an attribute we can mark these with that says "can't add or remove control flow dependencies", we should mark them that way.  </div><div><br></div><div>i don't see how you can resolve the "Things rely on memdep to give them clobbers for these and the lifetime markers" issue, without api changes.</div><div><br></div><div>A similar thing happens, BTW, with certain size accesses, where GVN relies on memdep to return a false clobber so it can perform load widening (see line 534).</div><div><br></div><div>If you want to somehow split up the API's so this makes sense, i'm not opposed.</div><div>But i don't think you can just change the behavior :)</div><div><br></div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 5, 2016 at 11:02 AM, Anna Thomas <span dir="ltr"><<a href="mailto:anna@azul.com" target="_blank">anna@azul.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">anna created this revision.<br>
anna added reviewers: sanjoy, reames, dberlin.<br>
anna added a subscriber: llvm-commits.<br>
<br>
Currently invariant.start intrinsic is treated as modifying memory. This<br>
patch changes invariant.start to be treated as read-only memory operation by MDA.<br>
This inturn helps passes such as GVN and memcpy optimizations to treat<br>
invariant.start as non-clobbering.<br>
<br>
<a href="https://reviews.llvm.org/D23214" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D23214</a><br>
<br>
Files:<br>
  lib/Analysis/<wbr>MemoryDependenceAnalysis.cpp<br>
  test/Transforms/<wbr>DeadStoreElimination/<wbr>invariant.start.ll<br>
  test/Transforms/GVN/invariant.<wbr>start.ll<br>
  test/Transforms/MemCpyOpt/<wbr>invariant.start.ll<br>
<br>
</blockquote></div><br></div>