[PATCH] D104654: [llvm][Inliner] Make PriorityInlineOrder lazily updated

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

In D104654#2832191 <https://reviews.llvm.org/D104654#2832191>, @taolq wrote:

> In D104654#2832141 <https://reviews.llvm.org/D104654#2832141>, @ChuanqiXu wrote:
>
>> How do you think about:
>>
>>   T pop() {
>>       if (!initialized) {
>>           make_heap ...
>>           initialized = true;
>>       }
>>   }
>>   // same as front
>>
>> It seems simple and cheaper.
>
> I think this if branch is not necessary. Because whenever pop() is called, the heap inside is maintained.
> So there is not necessary to do that.
>
>> Another option is:
>>
>>   SmallVector<CallBase*> CallSites;
>>   // Fill CallSites
>>   if (InlineEnablePriorityOrder)
>>       Calls = std::make_unique<PriorityInlineOrder>(CallSites); // Do make heap here
>>   else
>>       Calls = std::make_unique<InlinerOrder>(CallSites);
>>
>> It looks good too.
>
> Since this class is only used here, we could keep it minimal, no need to consider the generality.
> Even without this construct methrod, it could work.
>
> (Maybe I made some misunderstanding. The lazily update is for the priority of the call site which is pushed in heap,
> not for the heap. The heap is already well maintained.

Sorry for misunderstanding. The implementation looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104654