<div dir="ltr">Can you just expose a version of static bool instructionClobbersQuery(MemoryDef *MD, MemoryUse *MU, const MemoryLocOrCall &UseMLOC, AliasAnalysis &AA) {<div><br></div><div>That takes the mloc from the memory use?</div><div><br></div><div>IE</div><div>STH like:</div><div><br></div><div>bool  MemorySSA::defClobbersUse(MemoryDef *MD, MemoryUse *MU) {</div><div>   return instructionClobbersQuery(MD, MU, MemoryLocOrCall(MU), MSSA->AA);</div><div>}</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 11, 2016 at 8:30 AM, Sebastian Pop <span dir="ltr"><<a href="mailto:sebpop@gmail.com" target="_blank">sebpop@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">sebpop created this revision.<br>
sebpop added a reviewer: dberlin.<br>
sebpop added subscribers: george.burgess.iv, llvm-commits.<br>
<br>
This is a refreshed version of a patch that was reverted: it fixes<br>
the problems reported in both PR30216 and PR30499, and<br>
contains all the test-cases from both bugs.<br>
<br>
To hoist stores past loads, we used to search for potential<br>
conflicting loads on the hoisting path by following a MemorySSA<br>
def-def link from the store to be hoisted to the previous<br>
defining memory access, and from there we followed the def-use<br>
chains to all the uses that occur on the hoisting path. The<br>
problem is that the def-def link may point to a store that does<br>
not alias with the store to be hoisted, and so the loads that are<br>
walked may not alias with the store to be hoisted, and even as in<br>
the testcase of PR30216, the loads that may alias with the store<br>
to be hoisted are not visited.<br>
<br>
The current patch visits all loads on the path from the store to<br>
be hoisted to the hoisting position and uses the alias analysis<br>
to ask whether the store may alias the load. I was not able to<br>
use the MemorySSA functionality to ask for whether load and<br>
store are clobbered: I'm not sure which function to call, so I<br>
used a call to AA->isNoAlias().<br>
<br>
Store past store is still working as before using a MemorySSA<br>
query: I added an extra test to pr30216.ll to make sure store<br>
past store does not regress.<br>
<br>
Tested on x86_64-linux with check and a test-suite run.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D25476" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D25476</a><br>
<br>
Files:<br>
  llvm/lib/Transforms/Scalar/<wbr>GVNHoist.cpp<br>
  llvm/test/Transforms/GVNHoist/<wbr>pr30216.ll<br>
  llvm/test/Transforms/GVNHoist/<wbr>pr30499.ll<br>
<br>
</blockquote></div><br></div>