[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

Thu Feb 9 13:11:02 PST 2023

aemerson added a comment.

In D143624#4116114 <https://reviews.llvm.org/D143624#4116114>, @aeubanks wrote:

> In D143624#4116080 <https://reviews.llvm.org/D143624#4116080>, @aemerson wrote:
>
>> In D143624#4115986 <https://reviews.llvm.org/D143624#4115986>, @aeubanks wrote:
>>
>>> __clang_hip_math.hip is annoying...
>>>
>>> We'll need to remove the `MandatoryFirst` inliner in `ModuleInlinerWrapperPass`, although not sure if @mtrofin has any issues with that or not
>>>
>>> This isn't quite what I had initially thought, but this might be better. (I was thinking that we sort the calls in the inliner to visit alwaysinline calls first, but that might cause more compile time issues since we have to update the call graph after visiting all the calls in a function, but we might be visiting every function twice if we first batch process the alwaysinline calls then all other calls)
>>
>> I think that doesn't actually do the same thing as this, since the `Calls` vector is populated by visiting the functions in the current SCC. What we're trying to do with this patch is to ensure that all always-inline calls globally are processed first.
>
> That's true, but the legacy pass manager where the inliner explosion didn't happen in your case didn't process always-inline calls before other calls. So I don't think it's necessary to process alwaysinline calls globally first to fix your case. However, given that we still do two more rounds of inlining in the inliner pipeline after the alwaysinliner pass you added and your case still doesn't blow up, this solution does seem robust.

Sure, the exponential compile time case is actually just a side benefit here. The motivating reason for this change is actually to improve code size when building codebases that make heavy use of always_inline.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143624/new/

https://reviews.llvm.org/D143624