<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>"Hal Finkel" <hfinkel@anl.gov><br><b>Cc: </b>"Nick Lewycky" <nlewycky@google.com>, 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>, reviews+D7864+public+020798531ef64731@reviews.llvm.org<br><b>Sent: </b>Thursday, January 7, 2016 8:29:35 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">Finding minimal vertex separators is np-hard in general.<div><br></div><div>However, if we can prove the ssa graph for memoryssa (which is a single-phi world) is chordal, which i'm pretty sure it should be, we can find the minimal vertex separators in linear time:<br><a href="http://www.sciencedirect.com/science/article/pii/S0166218X98001231" target="_blank">http://www.sciencedirect.com/science/article/pii/S0166218X98001231</a><br></div><div><br></div><div>I'm pretty sure with a little modification, you can play the same games to find the property you are wanting.</div><div id="DWT605"><br></div></div></blockquote><br>I'll look at that, thanks!<br><br>Also, if I'm thinking about this correctly, we can do this in O(n*log(n)) time using essentially the existing technique. We know that the separator needs to dominate the original access. So, after the initial walk, you can filter out those visited blocks that dominate the original access. Starting from the original access and walking up, the relevant phis will all be vertex separators until you reach the ones that aren't. Thus, you can find the partitioning point by binary search. At each point in the search, you do another graph search, with the suspected vertex separator removed, and see if you reach none of the aliasing defs. All of the aliasing queries should be cached at that point, so the cost is just that of walking the subgraph.<br><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></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 7, 2016 at 5:25 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: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div><div style="font-family: arial,helvetica,sans-serif; font-size: 10pt; color: rgb(0, 0, 0);"><br><hr><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" <<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>><br><b>To: </b><a href="mailto:reviews%2BD7864%2Bpublic%2B020798531ef64731@reviews.llvm.org" target="_blank">reviews+D7864+public+020798531ef64731@reviews.llvm.org</a><br><b>Cc: </b>"Nick Lewycky" <<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>>, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>, <a href="mailto:anemet@apple.com" target="_blank">anemet@apple.com</a>, "Pete Cooper" <<a href="mailto:peter_cooper@apple.com" target="_blank">peter_cooper@apple.com</a>>, "vikram tarikere" <<a href="mailto:vikram.tarikere@gmail.com" target="_blank">vikram.tarikere@gmail.com</a>>, "Félix Cloutier" <<a href="mailto:felixcca@yahoo.ca" target="_blank">felixcca@yahoo.ca</a>>, "David Majnemer" <<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>>, "James Molloy" <<a href="mailto:james.molloy@arm.com" target="_blank">james.molloy@arm.com</a>>, "Philip Reames" <<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>>, "Sanjoy Das" <<a href="mailto:sanjoy@playingwithpointers.com" target="_blank">sanjoy@playingwithpointers.com</a>>, "llvm-commits" <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>><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<div><div class="h5"><br><br><div dir="ltr"><br><div class="gmail_extra"><br><div 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>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>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></div></div></blockquote>That makes sense, thanks!<span class=""><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"><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>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></span>Yep, looks the same in this regard.<span class=""><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>(You can see this in the upwardsdfswalk code that got removed:<br></div></div></div></div></blockquote></span>Indeed; the version from April recursed on phis to enforce this condition.<div><div class="h5"><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><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> </div></div></div></div></blockquote><br></div></div>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,<span class=""><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></span>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<span></span><br></div></span></div></div></blockquote></div><br></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>