This is an archive of the discontinued LLVM Phabricator instance.

[NFC][InlineCost] Simplify internal inlining cost interface
ClosedPublic

Authored by mtrofin on Dec 17 2019, 9:38 PM.

Details

Summary

All the use cases of CallAnalyzer use the same call site parameter to
both construct the CallAnalyzer, and then pass to the analysis member.
This change removes this duplication.

Diff Detail

Event Timeline

mtrofin created this revision.Dec 17 2019, 9:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 17 2019, 9:38 PM

There appear to be unrelated changes due to clang-format. I created https://reviews.llvm.org/D71646 to capture just those changes, and I plan to submit that first, then this one.

Jim added a subscriber: Jim.Dec 17 2019, 11:42 PM

It looks like some of changes are the same as this patch https://reviews.llvm.org/D71646.
Please based on https://reviews.llvm.org/D71646 this patch to make change.

Jim added a reviewer: Jim.Dec 17 2019, 11:43 PM
mtrofin updated this revision to Diff 234547.Dec 18 2019, 8:23 AM

rebase & rename

mtrofin retitled this revision from [llvm] Simplify internal inlining cost interface to [NFC][InlineCost] Simplify internal inlining cost interface.Dec 18 2019, 8:23 AM
mtrofin updated this revision to Diff 234551.Dec 18 2019, 8:29 AM

removed unrelated formatting in comments

There is comment like this:

/// The candidate callsite being analyzed. Please do not use this to do
/// analysis in the caller function; we want the inline cost query to be
/// easily cacheable. Instead, use the cover function paramHasAttr.
CallBase &CandidateCall;

Can you find out the history of this field?

On the other hand, if a different callsite is passed to analyzeCall other than CandidateCall, it will produce wrong results.

There is comment like this:

/// The candidate callsite being analyzed. Please do not use this to do
/// analysis in the caller function; we want the inline cost query to be
/// easily cacheable. Instead, use the cover function paramHasAttr.
CallBase &CandidateCall;

Can you find out the history of this field?

It seems it was introduced here: http://reviews.llvm.org/D9129. analyzeCall was already taking a CallSite parameter.

On the other hand, if a different callsite is passed to analyzeCall other than CandidateCall, it will produce wrong results.

That's the thing: the API is local to this file, and there is no case when we pass a different parameter to analyzeCall, than the parameter used to construct the CallAnalyzer object.

davidxl accepted this revision.Dec 19 2019, 3:09 PM

lgtm

This revision is now accepted and ready to land.Dec 19 2019, 3:09 PM
This revision was automatically updated to reflect the committed changes.