<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Sep 13, 2016 at 1:55 PM, 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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">> 1. No MemorySSA form handles *hoisting* stores (not gcc, not open64's<br>
> mu/chi/etc).<br>
> It requires building the reverse problem to make these links exist.<br>
<br>
</span>Would it be possible to provide an inverse MemorySSA graph:<br>
like a CFG df_walk has the idf_walk, except as you are saying,<br>
that for MemSSA we would have to build the inverse graph that<br>
is not just an upside-down version of MemSSA.<br></blockquote><div><br></div><div>Yes.</div><div><br></div><div>Consider:</div><div>load a</div><div>if () </div><div>  store a</div><div>else </div><div>  store a</div><div><br></div><div>SSU (as defined by the SSAPRE paper. There are at least 3 things called SSU in literature), would create:<br></div><div><br></div><div>load = use of mem1</div><div>mem2, mem3 = lambda (mem1)</div><div>if ()</div><div>  store = def of mem2</div><div>else</div><div>  store = def of mem3</div><div><br></div><div><br>Now you can at least recursively walk the uses of the stores and lambdas.</div><div><br></div><div>Right now you would have to walk the uses of the loads.</div><div><br></div><div><br></div><div>2. you just generate a form for WAR dependence.</div><div><br></div><div>This is "immediate upward use".</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">
<br>
MemSSA represents RAW and WAW dependences.<br>
The inverseMemSSA would represent WAR dependences.<br></blockquote><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="gmail-"><br>
> Instead, most things *sink* stores, which MemorySSA makes easy.<br>
> See SSUPRE, etc.<br>
><br>
> 2. If you are going to write your own walkers, etc, we don't make it easy<br>
> enough.<br>
><br>
> My initial view is: Rather than do this as a completely separate piece by<br>
> piece query interface, the intention was for folks to write custom walkers,<br>
> and use those.<br>
> For example, the code in this patch could be written as a MemorySSA walker<br>
> that implemented "getClobberingMemoryAccess" for stores.<br>
> It has a different definition of what it means to be a clobbering memory<br>
> access than the default walker.<br>
> That's okay.<br>
><br>
> *Inside of walkers*, we should make instructionClobbersQuery available.<br>
><br>
> Thoughts?<br>
<br>
</span>I'm not sure the MemSSA will have all the links to be able to walk<br>
the WAR dependences.<br></blockquote><div><br></div><div>It does not.</div><div><br></div><div>It's also not clear to me it should. AFAIK, most things simply common stores  by sinking rather than hoisting.</div><div><br></div><div>Obviously, you can do both because they don't cover each other (you can sink stores you can't hoist, you can hoist stores you can't sink), and you would need to do both to be optimal, code size wise.</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">
<br>
In the mean time, is the current patch suitable to commit to fix the miscompile?<br>
<br></blockquote><div><br></div><div>I'll review in a few moments. </div><div><br></div></div></div></div>