<html><head><style type='text/css'>p { margin: 0; }</style></head><body><div style='font-family: arial,helvetica,sans-serif; font-size: 10pt; color: #000000'><br><hr id="zwchr"><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><b>From: </b>"Daniel Berlin" <dberlin@dberlin.org><br><b>To: </b>reviews+D7864+public+020798531ef64731@reviews.llvm.org<br><b>Cc: </b>"Nick Lewycky" <nlewycky@google.com>, "Hal Finkel" <hfinkel@anl.gov>, anemet@apple.com, "Pete Cooper" <peter_cooper@apple.com>, "vikram tarikere" <vikram.tarikere@gmail.com>, "Félix Cloutier" <felixcca@yahoo.ca>, "David Majnemer" <david.majnemer@gmail.com>, "James Molloy" <james.molloy@arm.com>, "Philip Reames" <listmail@philipreames.com>, "Sanjoy Das" <sanjoy@playingwithpointers.com>, "llvm-commits" <llvm-commits@lists.llvm.org><br><b>Sent: </b>Thursday, January 7, 2016 6:52:57 PM<br><b>Subject: </b>Re: [PATCH] D7864: This patch introduces MemorySSA, a virtual SSA form for memory.Details on what it looks like are in MemorySSA.h<br><br><div dir="ltr"><br><div class="gmail_extra"><br><div id="DWT9729" class="gmail_quote">On Thu, Jan 7, 2016 at 4:48 PM, 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: 0px 0px 0px 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Thu, Jan 7, 2016 at 4:27 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</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;">hfinkel added inline comments.<br>
<br>
================<br>
Comment at: lib/Transforms/Utils/MemorySSA.cpp:1124<br>
@@ +1123,3 @@<br>
+ auto CurrAccessPair = UpwardsBFSWalkAccess(StartingAccess, Prev, Q);<br>
+ // Either we will have found something that conflicts with us, or we will have<br>
+ // hit the liveOnEntry. Check for liveOnEntry.<br>
----------------<br>
This algorithm in getClobberingMemoryAccess does not seem correct.</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;"> If we have this:<br>
<br>
[ entry ]<br>
|<br>
...<br>
|<br>
(clobbering access - a)<br>
...<br>
/ \<br>
... (clobbering access - b)<br>
| |<br>
... ....<br>
\ /<br>
...<br>
(starting access)<br>
<br>
UpwardsBFSWalkAccess seems to walk upward until it finds its first clobbering access, and returns that when found. In the case above, if we find clobbering access (b) first, then we're okay, because we'll pick some dominating def farther down the path which is on all paths to both dominating accesses. </blockquote><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<br>
But, if we hit clobbering access (a) first, then we'll accept that (incorrectly) as the new final access, because it dominates the original. However, not all paths to all aliasing defs terminate (or pass through) that access.<br></blockquote><div><br></div><div><br></div></span><div>So, note, in the above case, there *must* be a memory phi in the block of starting access, merging the two memory accesses. We should say that is the clobbering access.</div><div><br></div><div><br></div><div id="DWT9728">But you are correct, that this version, in the middle of refactoring, did not finish this rule correctly. The original algorithm only walked past a phi if all paths away from the phi reached the same clobbering access.</div></div></div></div></blockquote><br></div></div></div></blockquote>That makes sense, thanks!<br><br><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><div dir="ltr"><div class="gmail_extra"><div id="DWT9770" class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div><br></div><div id="DWT9769">This one should do the same.</div></div></div></div></blockquote><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div></div></div></div></blockquote></div></div></div></blockquote>Yep, looks the same in this regard.<br><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div></div></div></div></blockquote><div><br></div><div id="DWT9768">(You can see this in the upwardsdfswalk code that got removed:<br></div></div></div></div></blockquote>Indeed; the version from April recursed on phis to enforce this condition.<br><br><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div> // All the phi arguments should reach the same point if we can bypass</div><div>- // this phi. The alternative is that they hit this phi node, which</div><div>- // means we can skip this argument.</div><div>- if (FirstDef && (CurrentPair.first != PHIPair.first &&</div><div>- FirstDef != CurrentPair.first)) {</div><div>- ModifyingAccess = CurrAccess;</div><div>- break;</div><div>- } else if (!FirstDef) {</div><div>- FirstDef = CurrentPair.first; </div><div>)</div><div><br></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;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><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;"><br>
[entry]<br>
|<br>
.....<br>
(clobbering access - b)<br>
|<br>
.... ________________________________<br>
\ / |<br>
(x) |<br>
...... |<br>
| |<br>
| ______________________ |<br>
\ / | |<br>
(starting access) | |<br>
... | |<br>
(clobbering access - a) | |<br>
... | |<br>
| | | |<br>
| |______________________| |<br>
| |<br>
|_________________________________|<br>
<br></blockquote><div><br></div><div><br></div></span><div>Again, we will have a memory phi node here, and we will say the phi node is the clobbering access. We will walk one step from the phi, hit a, and say "we have a clobbering access."</div></div></div></div></blockquote><div><br></div><div>and by here i mean, you will have a memoryphi node in the block of startingaccess, and we should say that is the clobbering node.</div><div id="DWT9730"> </div></div></div></div></blockquote><br>Yep.<br><br>Out of curiosity, is there an efficient algorithm to solve the problem optimally? I suppose I could phrase it like this: Given some digraph G, a vertex S, and a set of verticies F, compute the set of vertex seperators between S and F, or more specifically, the vertex separator between S and F that has the largest distance from S.<br><br>Thanks again,<br>Hal<br><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div></div><br></div></div>
</blockquote><br><br><br>-- <br><div><span name="x"></span>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<span name="x"></span><br></div></div></body></html>