<div dir="ltr"><div><div>Cool!<br>Thank you for committing the patches!<br><br></div> Dario Domizioli<br></div> SN Systems - Sony Computer Entertainment<br><br><br><br></div><div class="gmail_extra"><br><br><div class="gmail_quote">
On 1 February 2014 10:45, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</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"><div class="gmail_extra"><div class="im"><br><div class="gmail_quote">On Fri, Jan 31, 2014 at 1:16 PM, Dario Domizioli <span dir="ltr"><<a href="mailto:dario.domizioli@gmail.com" target="_blank">dario.domizioli@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>Hm...<br><br>I had considered changing the calculation so that the
VectorBonus fraction was based on the number of non-simplified
instructions (i.e. NumInstructions - NumInstructionsSimplified), but
then I decided to go for a less risky change because I wasn't sure what
other kinds of instructions ended up in the "simplified" bucket. And furthermore some vector instructions may be simplified too, so the whole logic gets messy pretty quickly. The existence of a "hidden bonus" is itself quite odd (and
the fact that it didn't appear in the debug prints made the
investigation harder - hence my debug change too), so I'd understand if
you wanted to wait and properly rework the inline heuristic.<br>
<br>
</div><div>This specific change should only impact compilations with
debug information, and furthermore it would just realign their result to
what the correspondent non-debug compilation does. Non-debug builds are
already using the increased threshold and they inline all the functions
that would be affected by this change in debug builds, so this doesn't
technically increase our willingness to inline functions... it just
makes sure that we are coherent with and without -g. I see your general point about possible regressions though.<br>
<br>
</div><div>I don't know... I think this could work as an incremental change towards properly fixing the VectorBonus issues, but that's just my opinion.</div></blockquote></div><br></div>Yea, essentially that's the unhappy conclusion I came to (as well as Eric and others). Changing everything at once is too hard and the bug is indeed bad enough to warrant a quick fix. Applied your patch with a significantly simplified test case in r200609. (The test case simplifications are I think good because there is no need or desire for this debug information to emit as useful debug information, merely to clutter up the IR with it and ensure the optimizer does-the-right-thing.)</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">Thanks for the fix!</div><span class="HOEnZb"><font color="#888888"><div class="gmail_extra">-Chandler</div></font></span></div>
</blockquote></div><br></div>