<div dir="ltr">r301673<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 28, 2017 at 10:08 AM, 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">This is about to be done, as i have converted remaining users and the bootstraps are just about done.<div>I will follow up with the rev number.</div><div><br></div><div><div>For those needing to convert out of tree uses:<br></div><div><br></div><div>Trivially, you should be able to take any place that has SimplifyInstruction(I, ...), and do SimplifyInstruction(I, {...}).</div><div><br></div><div>However, where possible, it is going to be better to construct a SimplifyQuery once and use it everywhere.</div><div>CorrelatedValuePropagation and LoopRotation have examples of how to do this.</div><div><br></div><div>I have converted everywhere where it is easy to do <a href="http://so.to" target="_blank">so.to</a> construct them once, and everywhere else now constructs them using the above.</div><div><br></div><div>I am also introducing a helper function, named "getBestSimplifyQuery", that can take Pass/AnalysisManager/<wbr>LoopStandardAnalysis, and will construct the best SimplifyQuery possible from whatever optional analysis are available.</div><div><br></div><div>Thus, in a Pass runOnFunction, you could do</div><div><br></div><div>const SimplifyQuery SQ = getBestSimplifyQuery(*this, F)</div><div><br></div><div>and it will hand you back a structure filled in with as many of the analyses as are around and up to date. You can then pass that to SimplifyInstruction later.</div><div><br></div><div>Using this is strongly preferred as it will enable us to add analysis to SimplifyInstruction and just update those functions.</div><div><br></div><div>I bootstrapped with every target enabled, sorry if i missed something!</div><div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Apr 26, 2017 at 4:55 PM, Philip Reames via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<p>Agreed. This seems like overall a huge win for readability. <br>
</p>
<p>I am a bit worried about all the miscompiles you're going to
expose by using all that extra information, but we should
definitely just fix those. :)</p><span class="m_-7235669929099653478HOEnZb"><font color="#888888">
<p>Philip<br>
</p></font></span><div><div class="m_-7235669929099653478h5">
<br>
<div class="m_-7235669929099653478m_-7623849443690076453moz-cite-prefix">On 04/26/2017 08:53 AM, Hal Finkel via
llvm-dev wrote:<br>
</div>
<blockquote type="cite">
<p><br>
</p>
<div class="m_-7235669929099653478m_-7623849443690076453moz-cite-prefix">On 04/25/2017 11:32 PM, Daniel Berlin
via llvm-dev wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">For those not following along, startingin
r301379, Simplify* in InstructionSimplify now can just take a
query struct instead of 8000 optional arguments. Nothing is
really new since it used the same thing under the covers.
<div>
<div><br>
</div>
<div>I'm slowly converting the old uses away (deletion of
the old APIs is a different question).</div>
<div>Staring at most of them, i could just directly convert
them using braced list initialization, and construct the
objects again and again, but the vast majority of uses
actually:</div>
<div><br>
</div>
<div>A. don't change the relevant query arguments over the
lifetime of the pass</div>
<div>B. actually have more information sitting around than
they are passing along.</div>
<div><br>
</div>
<div>For example, instcombine has a DomTree and
AssumptionCache that are required, but it doesn't pass
them to SimplifyBinOp in a bunch of cases.</div>
<div><br>
</div>
<div>JumpThreading has TLI but doesn't pass it to
SimplifyCmpInst.</div>
<div><br>
</div>
<div>CVP at least admits it has a problem:</div>
<div>"CorrelatedValuePropagation.cp<wbr>p: // FIXME: Provide
TLI, DT, AT to SimplifyInstruction.</div>
<div>CorrelatedValuePropagation.cpp<wbr>: if (Value *V =
SimplifyInstruction(P, DL)) {</div>
<div>"</div>
<div>(This is because it uses LVI, which requires those
things, but it doesn't ask for them itself)</div>
<div><br>
</div>
<div>Assuming this is not deliberate, it would seem to me to
just be easiest to set up the query structure once in the
pass and use it everywhere, which would hopefully avoid
these issues in the future, besides making most of the
call strings dramatically shorter :)</div>
<div><br>
</div>
<div>Is there any real downside (compile time performance,
etc) to this vs what we do now (which is basically
constructing the query objects again and again under the
covers) that anyone can think of?</div>
<div><br>
</div>
<div>(again, assuming we are not deliberately avoiding
passing info to Simplify* in most of these cases)</div>
</div>
</div>
</blockquote>
<br>
I don't think it is deliberate; it is just that no one has done
the work to make it uniform. It is possible that doing this will
expose some compile-time issues, but if so, we should deal with
them for real (not just hide them by a failure to provide
optional-but-available analyses).<br>
<br>
I'm certainly in favor of more uniformity here.<br>
<br>
-Hal<br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div>
<div><br>
</div>
</div>
<div>I figured i'd ask before i went writing the scripts/etc
to help convert a bunch of this stuff :)</div>
<div><br>
</div>
</div>
<br>
<fieldset class="m_-7235669929099653478m_-7623849443690076453mimeAttachmentHeader"></fieldset>
<br>
<pre>______________________________<wbr>_________________
LLVM Developers mailing list
<a class="m_-7235669929099653478m_-7623849443690076453moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>
<a class="m_-7235669929099653478m_-7623849443690076453moz-txt-link-freetext" 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>
</pre>
</blockquote>
<br>
<pre class="m_-7235669929099653478m_-7623849443690076453moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
<br>
<fieldset class="m_-7235669929099653478m_-7623849443690076453mimeAttachmentHeader"></fieldset>
<br>
<pre>______________________________<wbr>_________________
LLVM Developers mailing list
<a class="m_-7235669929099653478m_-7623849443690076453moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>
<a class="m_-7235669929099653478m_-7623849443690076453moz-txt-link-freetext" 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>
</pre>
</blockquote>
<br>
</div></div></div>
<br>______________________________<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" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br>
<br></blockquote></div><br></div></div></div></div>
</blockquote></div><br></div>