<div dir="ltr"><div>When you say that distribution shouldn't be used, do you mean within instcombine rather than some other pass? Or not all as an IR optimization?<br><br></div><div>A dedicated optimization pass that looks for and makes factoring/distribution folds to eliminate instructions seems like it would solve the problems that I'm seeing. <br>Ie, I'm leaning towards the proposal here: <a href="https://reviews.llvm.org/D41574" target="_blank">https://reviews.llvm.org/D4157<wbr>4</a><br></div><div>But I'd make it less "weak". :)<br><br>Ideally, that allows removing duplicate functionality from instcombine, 
but I'm guessing that it will take a lot of work to cut that cord without causing regressions. Note that instcombine even has a 
specialized reassociation pass-within-a-pass for fadd/fsub alone:<br><a href="https://reviews.llvm.org/rL170471" target="_blank">https://reviews.llvm.org/rL170<wbr>471</a><br></div><div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 8, 2018 at 9:22 PM, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@nondot.org" target="_blank">clattner@nondot.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><br><div><span><br><blockquote type="cite"><div>On May 8, 2018, at 9:50 AM, Daniel Berlin via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="m_3383979024771043645m_-3549169013129636236Apple-interchange-newline"><div><div dir="ltr">1.  The reassociate pass that exists right now was *originally* (AFAIK) written to enable CSE/GVN to do better.</div></div></blockquote><div><br></div></span><div>Agreed.  The original mindset included a (naive) belief that going with a canonical form was better than teaching redundancy elimination to handle abstractions (as a matter of separation of concerns).  I think that that was a failed experiment at this point :-).  We should still canonicalize x*x*x into powi, but the more aggressive canonicalization (like distribution) shouldn’t be used IMO.</div><span class="m_3383979024771043645HOEnZb"><font color="#888888"><div><br></div><div>-Chris</div><div><br></div><div><br></div><br></font></span><blockquote type="cite"><div><span><div dir="ltr"> That particular issue is solvable in other ways, because there are good ways to integrate reassociation into CSE/GVN (and at this point, it would probably be cheaper than -reassociate since it would modify code less, only changing it when there are actual redundancies )<div><br></div><div><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration-style:initial;text-decoration-color:initial">I don't know what other things people are trying to target with reassociate these days, it would be good to understand what others see as the use cases.</div><br></div><div>My only other fear with removing it is that we have a tendency to try to make every optimization do everything, so i fear removing reassociate might just have people try to improve every pass to do reassociation instead of declaring what is good enough.</div><div><br></div><div>(For example, GVN already does some reassociation, as do some analysis. Other analysis relies more on a canonical form existing and don't, for example, try to perform commutativity on their own.  IMHO, we should just declare one or the other to be the case, and hold that line, rather than generate a mishmash)</div><div><br></div><div><br></div><div><div>2. "<span style="font-size:12.8px">Keep in mind that instcombine runs 1st, runs to fixed-point, and runs 8 times in the -O2 pipeline."</span></div><div><br></div><div><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial"><span style="font-size:12.8px">Y'all know how i feel about this one, so i'll leave it alone :)</span></div><br class="m_3383979024771043645m_-3549169013129636236gmail-Apple-interchange-newline"></div></div><br><div><br></div><div>--Dan</div></div></span><span>
______________________________<wbr>_________________<br>LLVM Developers mailing list<br><a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br></span></div></blockquote></div><br></div></blockquote></div><br></div></div></div></div></div>