<div dir="ltr"><a href="http://reviews.llvm.org/D8576">http://reviews.llvm.org/D8576</a> generalizes the interface to isDivergent(Value *) and isUniform(Value *). PTAL. <div><br><div class="gmail_quote">On Fri, Apr 3, 2015 at 10:36 AM Jingyue Wu <<a href="mailto:jingyue@google.com">jingyue@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote">On Fri, Apr 3, 2015 at 6:28 AM Fernando Magno Quintao Pereira <<a href="mailto:fernando@dcc.ufmg.br" target="_blank">fernando@dcc.ufmg.br</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Jingyue.<br>
<br>
I went over your code today. It looks very nice to me. I believe<br>
the programming style is very clear, and the analysis seem to be<br>
correct to me. I am sending you below a few observations.<br>
<br>
Regards,<br>
<br>
Fernando<br>
<br>
---<br>
<br>
1) I think that the more recent TOPLAS paper, "Divergence Analysis -<br>
Sampaio, Souza, Collange, Pereira, 2013", is a better source of<br>
information than our original PACT publication (for the comment in<br>
lines 31-33).<br></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>Ack'ed. Will fix. </div></div></div><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
2) Would it be possible to have a function such as "bool<br>
isUniformValue(const Value *V) const" or (just the same) "bool<br>
isDivergentValue(const Value *V) const" in the public interface of the<br>
analysis? In fact, this is more general than "isDivergentBranch"<br>
(lines 107-109), and it could help the register allocator, for<br>
instance.<br></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>Agreed. The only concern I have is we need to keep all divergent instructions (as opposed to all divergent branches) for the users of the divergence analysis. I think it's fine given this analysis already spends this amount of space when computing divergent values. </div></div></div><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
3) For non-structured codes, is it not possible that your method<br>
exploreSyncDependency may cause the same node to be visited more than<br>
once? Imagine a CFG that is like a butterfly, like the one in the PDF<br>
I sent you. See line 290. You could avoid this by interrupting the<br>
search once you visit a divergent instruction.<br></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>I don't get this. Both exploreSyncDependency and exploreDataDependency check the visited flag of a value before adding it to the work list. I think this is enough for ensuring each value is explored at most once. </div></div></div><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
4) I think you are flagging a variable that is divergent outside a<br>
loop as divergent wherever it is alive, even if it is uniform inside<br>
the loop. That is not wrong, but it may lead to less precise results.<br>
Imagine, for instance:<br>
<br>
int i = 0;<br>
while (i < tid) {<br>
i++<br>
if (i % 0) { // i is uniform for every active thread<br>
int x = 2 * i; // x is uniform for every active thread<br>
v[tid] = x;<br>
}<br>
}<br>
v[tid] = i; // i is divergent for every thread<br>
<br>
A more precise analysis would split the live range of i right after<br>
the loop, and you would end up with something like:<br>
<br>
int i = 0;<br>
while (i < tid) {<br>
i++<br>
if (i % 0) {<br>
int x = 2 * i;<br>
v[tid] = x;<br>
}<br>
}<br>
i0 = phi(i); // i0 is divergent, for it is used outside the influence region.<br>
v[tid] = i0;<br>
<br></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>This is a very good point, but I need to mention two things. </div><div>1. My implementation does not mark the i in your example as divergent; it only marks its out-of-the-loop users as divergent. Therefore, if you worry about whether x is considered divergent, the answer is no. </div><div>2. Divergence analysis is supposed to be readonly and shouldn't modify the program. However, if we want to split the live range of i for a more precise model, we can run LoopSimplify+LCSSA before the divergence analysis. LCSSA (<a href="http://llvm.org/docs/doxygen/html/LCSSA_8cpp_source.html" target="_blank">http://llvm.org/docs/doxygen/html/LCSSA_8cpp_source.html</a>) in particular will rewrite all out-of-loop users to a PHI node with a single incoming value (see the header comments in LCSSA.cpp). I believe when running on an LCSSA form, my current implementation can distinguish the two live ranges. I'll double check that. </div><div>P.S. LCSSA only transforms natural loops. </div></div></div><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
On 4/3/15, Jingyue Wu <<a href="mailto:jingyue@google.com" target="_blank">jingyue@google.com</a>> wrote:<br>
> Hi Fernando,<br>
><br>
> I fixed the sync dependency computation in this update. I used to consider<br>
> only the if-then-else case; this version accounts for the loop case.<br>
><br>
> Please take a look when you have time. Thanks a lot for your help!<br>
><br>
> Jingyue<br>
><br>
> ---------- Forwarded message ---------<br>
> From: Jingyue Wu <<a href="mailto:jingyue@google.com" target="_blank">jingyue@google.com</a>><br>
> Date: Thu, Apr 2, 2015 at 10:48 PM<br>
> Subject: Re: [PATCH] Divergence analysis for GPU programs<br>
> To: <<a href="mailto:jingyue@google.com" target="_blank">jingyue@google.com</a>>, <<a href="mailto:resistor@mac.com" target="_blank">resistor@mac.com</a>>, <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>, <<br>
> <a href="mailto:eliben@google.com" target="_blank">eliben@google.com</a>>, <<a href="mailto:meheff@google.com" target="_blank">meheff@google.com</a>>, <<a href="mailto:justin.holewinski@gmail.com" target="_blank">justin.holewinski@gmail.com</a>><br>
> Cc: <<a href="mailto:bjarke.roune@gmail.com" target="_blank">bjarke.roune@gmail.com</a>>, <<a href="mailto:madhur13490@gmail.com" target="_blank">madhur13490@gmail.com</a>>, <<br>
> <a href="mailto:thomas.stellard@amd.com" target="_blank">thomas.stellard@amd.com</a>>, <<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>>, <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>>, <<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>><br>
><br>
><br>
> This update fixes sync dependency computation. If a value is used outside<br>
> of<br>
> the loop in that it is defined, the user is sync dependent on the exit<br>
> condition of the loop.<br>
><br>
><br>
> <a href="http://reviews.llvm.org/D8576" target="_blank">http://reviews.llvm.org/D8576</a><br>
><br>
> Files:<br>
> include/llvm/Analysis/Passes.<u></u>h<br>
> include/llvm/Analysis/<u></u>TargetT<u></u>ransformInfo.h<br>
> include/llvm/Analysis/<u></u>TargetT<u></u>ransformInfoImpl.h<br>
> include/llvm/CodeGen/<u></u>BasicTTI<u></u>Impl.h<br>
> include/llvm/<u></u>InitializePasses.<u></u>h<br>
> include/llvm/LinkAllPasses.h<br>
> lib/Analysis/Analysis.cpp<br>
> lib/Analysis/CMakeLists.txt<br>
> lib/Analysis/<u></u>DivergenceAnalys<u></u>is.cpp<br>
> lib/Analysis/<u></u>TargetTransformI<u></u>nfo.cpp<br>
> lib/Target/NVPTX/<u></u>NVPTXTargetT<u></u>ransformInfo.cpp<br>
> lib/Target/NVPTX/<u></u>NVPTXTargetT<u></u>ransformInfo.h<br>
> test/Analysis/<u></u>DivergenceAnaly<u></u>sis/NVPTX/<u></u>diverge.ll<br>
> test/Analysis/<u></u>DivergenceAnaly<u></u>sis/NVPTX/lit.<u></u>local.cfg<br>
><br>
> EMAIL PREFERENCES<br>
> <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/<u></u>setti<u></u>ngs/panel/<u></u>emailpreferences/</a><br>
><br>
</blockquote></div></div></blockquote></div></div></div>