<div dir="ltr">I haven't updated the patch. I talked to Chandler offline and I'm modifying my patch based on that conversation. Will upload the updated version soon.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 8, 2016 at 6:12 PM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Did you forget to update the patch ?<br>
<span class="HOEnZb"><font color="#888888"><br>
David<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Mon, Aug 8, 2016 at 5:53 PM, Easwaran Raman <<a href="mailto:eraman@google.com">eraman@google.com</a>> wrote:<br>
> eraman marked 2 inline comments as done.<br>
><br>
> ================<br>
> Comment at: lib/Transforms/IPO/<wbr>InlineSimple.cpp:79-81<br>
> @@ +78,5 @@<br>
> +<br>
> +  // Generate the parameters to tune the inline cost analysis based on<br>
> +  // commandline options.<br>
> +  InlineParams getInlineParams(int Threshold) {<br>
> +    InlineParams Params;<br>
> ----------------<br>
> chandlerc wrote:<br>
>> Sorry, I guess I wasn't clear before...<br>
>><br>
>> I'm specifically suggesting keeping all of this logic inside of InlineCost.cpp as how the *default* parameters get set.<br>
>><br>
>> If we need the ability to get custom parameters which still reflect the command line flags, there should be some API in InlineCost.h that the inliner queries to get the necessary params.<br>
>><br>
>><br>
>> My goal here stems from two related principles:<br>
>> - The tunable flags and the constants that interact are are fundamentally in the same unit domain should live in a single place (InlineCost.{h,cpp}).<br>
>> - The inliner *passes* should have no real logic to compute thresholds, they should call well documented APIs provided by the cost layer.<br>
>><br>
>> A minor secondary goal -- the inliner shouldn't have to do extra work to get the defaults. Those should just be used. It is when an inliner pass wants to deviate from the defaults that it should take explicit steps and call explicit flags.<br>
> Thanks for the clarification. I have moved getInlineParams and other associated logic that computes the threshold from the flags to InlineCost.cpp. SimpleInliner now calls one of these methods to get an InlineParams object which gets passed to getInlineCost.<br>
><br>
><br>
> <a href="https://reviews.llvm.org/D22120" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D22120</a><br>
><br>
><br>
><br>
</div></div></blockquote></div><br></div>