<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Apr 27, 2017 at 4:14 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: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 class="">On Thu, Apr 27, 2017 at 4:02 PM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.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"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="m_6955511231093651489gmail-">On Thu, Apr 27, 2017 at 2:38 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, Apr 27, 2017 at 1:42 PM, Wei Mi via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.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"><span class="m_6955511231093651489gmail-m_8761945199375511617m_-6293878154267179778gmail-">On Thu, Apr 27, 2017 at 1:26 PM, Daniel Berlin via Phabricator<br>
<<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br>
> dberlin added a comment.<br>
><br>
> Isn't this close to what n-ary reassociate does?<br>
> (if so, why not just use that and disable this?)<br>
><br>
<br>
</span>Nary reassociate tries to maximize the redundency exposed by<br>
reassociation. </blockquote><div><br></div></span><div>As does reassociate :)</div><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This patch tries to reduce the register pressure by<br>
changing where to insert the intermediate results of reassociation,<br>
and it doesn't try to improve the result of reassociation for better<br>
CSE/PRE/LICM optimizations. </blockquote><div><br></div></span><div>I understand.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The two have different motivations, so I<br>
don't see how nary reassociation can be helpful here.<br></blockquote><div><br></div></span><div>The placement algorithm for computations that n-ary reassociate uses tries to reuse existing computation points.</div><div>This patch tries to fix  "<span style="color:rgb(0,0,0);font-family:"segoe ui","segoe ui emoji","segoe ui symbol",lato,"helvetica neue",helvetica,arial,sans-serif;font-size:13px">too many overlapped life-ranges.</span><span style="color:rgb(0,0,0);font-family:"segoe ui","segoe ui emoji","segoe ui symbol",lato,"helvetica neue",helvetica,arial,sans-serif;font-size:13px"> "</span></div><div><br></div><div>The algorithm it uses, to find nearest common dominator of operands, etc, is pretty much what n-ary reassociate does:</div><div>"</div><div><div>  // Look for the closest dominator LHS of I that computes LHSExpr, and replace</div><div>  // I with LHS op RHS.</div></div><div>"</div><div><br></div></div></div></div></blockquote><div><br></div><div><br></div></span><div>It seems to me that this may not necessarily result in reduced overlapping live ranges -- it  can increase the live range of the temporary that holds the value of LHS. </div></div></div></div></blockquote></span><div>Sometimes, yes.</div><div>But you could also fix this. </div><span class=""><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> What this patch, on the other hand, can almost always result in non-increased register pressure.</div></div></div></div></blockquote><div><br></div></span><div>Maybe?<br></div><div class="gmail_quote"><br></div>Remember that this reassociate has *zero* global view of expressions.</div><div class="gmail_quote">It only makes locally good choices.</div><div class="gmail_quote"><br></div><div class="gmail_quote">IE given:<br><div class="gmail_quote">     bar(a + b);</div><div class="gmail_quote">     bar((a + 2) + b);</div><div class="gmail_quote"><br></div><div class="gmail_quote"><br></div><div class="gmail_quote">This reassociate will process the a + b and a + 2 + b separately,</div><div class="gmail_quote"><br></div><div class="gmail_quote">You are assuming the local choices you make about placement in such case turn out globally good, or even globally better.</div><div class="gmail_quote"><br></div><div class="gmail_quote">You already know i can prove this untrue :)</div><div class="gmail_quote"><br></div></div></div></div></blockquote><div><br></div><div><br></div><div>I might be unclear. </div><div><br></div><div><br></div><div>IIUC, this patch does intend to change the logic of reassociation which means the reassociation decisions will remain unchanged. What changed is where the sub-expressions (of the the root tree) are placed. The end result is that the live ranges of the leaves of the tree are guaranteed to be shrinked (or remain unchanged).  The intermediate value produced by the subtree may increased live range, but given that they have single use, the overall register pressure should be reduced. As I said, it is a way to tame the code motion introduced.</div><div><br></div><div>David</div><div> </div></div><br></div></div>