<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jan 22, 2020 at 1:10 PM Michael Holman <<a href="mailto:Michael.Holman@microsoft.com">Michael.Holman@microsoft.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div lang="EN-US">
<div class="gmail-m_1455608269849742561WordSection1">
<p class="MsoNormal">The new flag isn’t only a Boolean and the value of --inline-threshold is not ignored. The value is used here:<u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:8pt;font-family:Consolas;color:rgb(43,145,175)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:8pt;font-family:Consolas;color:rgb(43,145,175)">InlineParams</span><span style="font-size:8pt;font-family:Consolas;color:black"> llvm::getInlineParams(</span><span style="font-size:8pt;font-family:Consolas;color:blue">int</span><span style="font-size:8pt;font-family:Consolas;color:black">
</span><span style="font-size:8pt;font-family:Consolas;color:gray">Threshold</span><span style="font-size:8pt;font-family:Consolas;color:black">) {<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:8pt;font-family:Consolas;color:black">...</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:8pt;font-family:Consolas;color:black">
</span><span style="font-size:8pt;font-family:Consolas;color:blue">if</span><span style="font-size:8pt;font-family:Consolas;color:black"> (InlineThreshold.getNumOccurrences() > 0)<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:8pt;font-family:Consolas;color:black"> Params.DefaultThreshold = InlineThreshold;<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:8pt;font-family:Consolas;color:black">
</span><span style="font-size:8pt;font-family:Consolas;color:blue">else</span><span style="font-size:8pt;font-family:Consolas;color:black"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:8pt;font-family:Consolas;color:black"> Params.DefaultThreshold =
</span><span style="font-size:8pt;font-family:Consolas;color:gray">Threshold</span><span style="font-size:8pt;font-family:Consolas;color:black">;</span><u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">If --inline-threshold is specified, this code overrides the other 2 uses of InlineThreshold that I changed to DefaultThreshold, so there is no change to the behavior of --inline-threshold either.</p></div></div></blockquote><div><br></div><div>Sorry, I completely missed this use. Then it seems ok to me. But please do add a test.</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 lang="EN-US"><div class="gmail-m_1455608269849742561WordSection1"><p class="MsoNormal"><u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><b>From:</b> Teresa Johnson <<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>> <br>
<b>Sent:</b> Wednesday, January 22, 2020 1:00 PM<br>
<b>To:</b> <a href="mailto:reviews%2BD73217%2Bpublic%2B020daf5bf27953b8@reviews.llvm.org" target="_blank">reviews+D73217+public+020daf5bf27953b8@reviews.llvm.org</a><br>
<b>Cc:</b> Michael Holman <<a href="mailto:Michael.Holman@microsoft.com" target="_blank">Michael.Holman@microsoft.com</a>>; Easwaran Raman <<a href="mailto:eraman@google.com" target="_blank">eraman@google.com</a>>; Mircea Trofin <<a href="mailto:mtrofin@google.com" target="_blank">mtrofin@google.com</a>>; David Li <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>>; Aditya Kumar <<a href="mailto:hiraditya@msn.com" target="_blank">hiraditya@msn.com</a>>; <a href="mailto:haicheng@codeaurora.org" target="_blank">haicheng@codeaurora.org</a>; llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>><br>
<b>Subject:</b> [EXTERNAL] Re: [PATCH] D73217: [InlineCost] Add flag to allow changing the default inline cost<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On Wed, Jan 22, 2020 at 12:51 PM David Li via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">davidxl added a comment.<br>
<br>
ok. I see the intention. As long as the behavior of --inline-threshold option is not changed (it still overrides the new option), the new option seems fine to me. Adding individually controlled option for size opt seems a good idea too.<u></u><u></u></p>
</blockquote>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">It will affect the behavior of -inline-threshold as the patch currently stands (the option's value is essentially ignored). That's why I'm suggesting an alternative to leave -inline-threshold as is, and add a new bool flag e.g. -inline-threshold-overrides-opt-size,
probably defaulted to true to keep current behavior.<u></u><u></u></p>
</div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal" style="margin-bottom:12pt"><br>
<br>
Repository:<br>
rG LLVM Github Monorepo<br>
<br>
CHANGES SINCE LAST ACTION<br>
<a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD73217%2Fnew%2F&data=02%7C01%7Cmichael.holman%40microsoft.com%7C4474af52fe8a40eb37aa08d79f7e1801%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637153236229634708&sdata=ELe%2FaJEYK9acYX7z8iEp%2FaqU9ZBB3pJvAQ7tW3XqHEc%3D&reserved=0" target="_blank">
https://reviews.llvm.org/D73217/new/</a><br>
<br>
<a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD73217&data=02%7C01%7Cmichael.holman%40microsoft.com%7C4474af52fe8a40eb37aa08d79f7e1801%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637153236229634708&sdata=ihUnpKdxBuIpNCThb%2BFymTzGDB7QuDJBuAI2sg%2FF15Q%3D&reserved=0" target="_blank">https://reviews.llvm.org/D73217</a><br>
<br>
<br>
<u></u><u></u></p>
</blockquote>
</div>
<p class="MsoNormal"><br clear="all">
<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<p class="MsoNormal">-- <u></u><u></u></p>
<div>
<div>
<div>
<table border="0" cellspacing="0" cellpadding="0">
<tbody>
<tr>
<td nowrap style="border-right:none;border-bottom:none;border-left:none;border-top:1.5pt solid rgb(213,15,37);padding:0in">
<p class="MsoNormal"><span style="font-size:12pt;font-family:Arial,sans-serif;color:rgb(85,85,85)">Teresa Johnson |<u></u><u></u></span></p>
</td>
<td nowrap style="border-right:none;border-bottom:none;border-left:none;border-top:1.5pt solid rgb(51,105,232);padding:0in">
<p class="MsoNormal"><span style="font-size:12pt;font-family:Arial,sans-serif;color:rgb(85,85,85)"> Software Engineer |<u></u><u></u></span></p>
</td>
<td nowrap style="border-right:none;border-bottom:none;border-left:none;border-top:1.5pt solid rgb(0,153,57);padding:0in">
<p class="MsoNormal"><span style="font-size:12pt;font-family:Arial,sans-serif;color:rgb(85,85,85)"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |<u></u><u></u></span></p>
</td>
<td nowrap style="border-right:none;border-bottom:none;border-left:none;border-top:1.5pt solid rgb(238,178,17);padding:0in">
</td>
</tr>
</tbody>
</table>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top:2px solid rgb(213,15,37)">Teresa Johnson |</td><td nowrap style="border-top:2px solid rgb(51,105,232)"> Software Engineer |</td><td nowrap style="border-top:2px solid rgb(0,153,57)"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top:2px solid rgb(238,178,17)"><br></td></tr></tbody></table></span></div></div></div></div>