<br><br><div class="gmail_quote"><div dir="ltr">On Thu, May 25, 2017, 2:43 PM Chandler Carruth via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">chandlerc marked an inline comment as done.<br>
chandlerc added a comment.<br>
<br>
In <a href="https://reviews.llvm.org/D33540#764460" rel="noreferrer" target="_blank">https://reviews.llvm.org/D33540#764460</a>, @tejohnson wrote:<br>
<br>
> > I hoisted the ThinLTO stop point pre-link above the partial inliner and the RPO function attr inference. Partial inlining isn't going to make the code smaller and it seems better to just do that once post-link.<br>
><br>
> Added davidxl to comment on this change. It seems like doing some partial inlining during the pre-link for ThinLTO will have an effect on the importing. Also, it is pre-link (as well as post-link) in the old pass pipeline, so this would introduce an inconsistency between the pass pipelines. May be better to keep it in the same places, and evaluate the effect of removing it from the pre-link pass pipeline separately?<br>
<br>
<br>
Well, partial inliner isn't run at all yet. So my hope was it can get moved around later.<br>
<br>
I somewhat wanted to make the split line up with the simplification split that is already there in the pipeline. But if you feel strongly we need to keep these exactly the same, I can essentially copy the extra passes into the tail of the thinlto prelink stuff.<br>
<br>
<br>
<br>
================<br>
Comment at: lib/Passes/PassBuilder.cpp:708<br>
+ModulePassManager<br>
+PassBuilder::buildThinLTOPreLinkDefaultPipeline(OptimizationLevel Level,<br>
+                                                bool DebugLogging) {<br>
----------------<br>
tejohnson wrote:<br>
> I don't see this getting invoked.<br>
Yeah, the original patch had a change to Clang, but I wanted to get a submittable LLVM-focused version.<br></blockquote></div><div><br></div><div>Dehao just noticed that it still isn't be invoked, other than from the manual pipeline setup for testing. Do you still have your clang side changes around that you could submit? Or let me know if you prefer I add that.</div><div>Teresa</div><div><br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I'll add support for parsing these pipelines much like we support parsing 'default<O2>' and such so that we have some coverage of stuff.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D33540" rel="noreferrer" target="_blank">https://reviews.llvm.org/D33540</a><br>
<br>
<br>
<br>
</blockquote></div>