<div dir="ltr">> There are a couple of FIXME comments in this change that identify cases where MemorySSA is maybe being too conservative (e.g. when dealing with fence release instructions and load atomic instructions). <div><br></div><div>Yup! D16875 that should make MemSSA more willing to handle atomics/volatile ops. Philip gave it a verbal LGTM*, but IIRC he thought it may be better to wait until we get MSSA integrated into a few passes (to get any big bugs worked out) before we start making it super aggressive with memory ordering. I have no opinion on when it should land; if we want it in now, I'm happy to commit it today.</div><div><br></div><div>> <span style="font-size:12.8px">Similarly, what do you think of Phillip's suggestion to look at using ValueHandles in MemorySSA to make removal invalidating more automated?</span></div><br style="font-size:12.8px"><div>I was planning for that to come with the overhauled cache (most recent attempt is abandoned at D19503). I should have more time to take another stab at it this weekend. If not, I'll at least end up tweaking the current implementation to use VHes.</div><div><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">* - (To be as clear as I can be, it was an LGTM with comments. Addressing the comments required refactoring the code a bit. I'm assuming the refactoring doesn't matter, and the LGTM still applies.)</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 17, 2016 at 9:12 AM, Daniel Berlin <span dir="ltr"><<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Tue, May 17, 2016 at 8:23 AM, Geoff Berry <span dir="ltr"><<a href="mailto:gberry@codeaurora.org" target="_blank">gberry@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">gberry added a comment.<br>
<br>
@reames I've attempted to resolved most of your individual concerns (or at least made them explicit in the change).  The bigger question of whether this is worth the compile time remains to be determined.  Do you think more tests need to be added in addition to the already existing EarlyCSE tests?  Adding additional run lines to those tests to enable -early-cse-use-memoryssa seems like overkill to me, but I don't feel to strongly about it.  Or are you more concerned about adding new tests for cases that are only caught by MemorySSA (both positive and negative)?<br>
<br>
@dberlin, @george.burgess.iv There are a couple of FIXME comments in this change that identify cases where MemorySSA is maybe being too conservative (e.g. when dealing with fence release instructions and load atomic instructions). </blockquote><div><br></div></span><div>This is known :)</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Do you think it is reasonable to refine these cases in MemorySSA</blockquote><div><br></div></span><div>Yes, it's one of george's goals.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">or is the conservatism restricted to EarlyCSE's usage, in which case we should deal with it in EarlyCSE?</blockquote><div><br></div></span><div>No, we should make memoryssa work well.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">  Similarly, what do you think of Phillip's suggestion to look at using ValueHandles in MemorySSA to make removal invalidating more automated?<br>
<br></blockquote><div><br></div></span><div>Already suggested this to george a few weeks ago.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<a href="http://reviews.llvm.org/D19821" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19821</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>
</blockquote></div><br></div></div>