<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 3, 2017 at 10:45 PM, Sanjoy Das via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.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">sanjoy added inline comments.<br>
<br>
<br>
================<br>
Comment at: lib/Transforms/Scalar/LoopStre<wbr>ngthReduce.cpp:3430<br>
<span class="gmail-m_2193190211439086712gmail-">+  if (AddOps.size() >= 5 && LU.AllFixupsOutsideLoop)<br>
+    return;<br>
+<br>
</span>----------------<br>
I'm not too familiar with LSR, but this looks pretty ad-hoc -- why can't the same compile-time blowup happen for an `LSRUse` with `AllFixupsOutsideLoop` = `false`?<br>
<div class="gmail-m_2193190211439086712gmail-HOEnZb"><div class="gmail-m_2193190211439086712gmail-h5"><br></div></div></blockquote><div><br></div><div>In theory reassocation can also cause candidate formulae to blow up for LSRUse with AllFixupsOutsideLoop = false. I only handle LSRUses with AllFixupsOutsideLoop = true because they should have less impact on the the final LSR result when we put a reassociation cap.  </div><div><br></div><div>Because it is not a common problem and we havn't seen real case involving LSRUse with AllFixupsOutsideLoop = false suffering from it, I choose to limit the range affected by the change. </div><div><br></div><div>Thanks,</div><div>Wei.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-m_2193190211439086712gmail-HOEnZb"><div class="gmail-m_2193190211439086712gmail-h5">
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D30350" rel="noreferrer" target="_blank">https://reviews.llvm.org/D3035<wbr>0</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div></div>