[PATCH] D104654: [llvm][Inliner] Make PriorityInlineOrder lazily updated
Chuanqi Xu via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 22 01:14:40 PDT 2021
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
More information about the llvm-commits
mailing list