DisallowAllBonuses is called only when the callsite or the callee entry is cold. The only reason to make it conditional on caller's inlinability would be for code size improvement. Among the 3 bonuses disabled in DisallowAllBonuses, only the LastCallToStaticBonus is useful to allow for code size reduction. You don't want to give a single BB bonus or a vector bonus for a cold callee (or a callee at a cold callsite) whose caller is unlikely to be inlined.

LGTM

LGTM

The only case where not inlining results in a bigger caller size is when the cost of inlining a callsite is negative. But such callsites always get inlined as the threshold is always positive. Why do you require an additional bonus to get those callsites inlined?

Instead of passing a boolean, would it be better to pass ProfileCountType enum? Right now it doesn't make a difference as there are only two valid profile count types, but if that set expands, we could make them like bitmasks and then get a subset match.

LGTM after making the hasProfileSummary check consistent in both flavors of shouldOptimizeForSize

One option is to call DiagnosticHandler::isMissedOptRemarkEnabled in Inliner and pass the result (bool) when creating the CallAnalyzer object. This can be ORed with whether the missed opt remark is enabled for inline-cost.

Also defined AddCountTy to be function_ref<void(NodeRef, Scaled64)>

Address review comments

In D34919#1346759, @eraman wrote:

Reviving this old patch as Wei recently noticed a case where a large function with many callees and many callers spent significant amount of time in shouldBeDeferred. This was triggered by change in profile causing many callees to be inlined into this large function.

Fix variable naming.

In D34919#1346838, @chandlerc wrote:

Nice, LGTM!

Does it make sense to add a (very early) assert that the thresholds are non-negative? I agree with you that it shouldn't be happening and it seems like a good invariant to enforce. Can of course be a follow-up patch.

Reviving this old patch as Wei recently noticed a case where a large function with many callees and many callers spent significant amount of time in shouldBeDeferred. This was triggered by change in profile causing many callees to be inlined into this large function.

Rebase.

LGTM

Add a test case to check for HasSyntheticEntryCounts flag.

Add a test case to check for synthetic entry counts and rebase.

Could you add a test case or augment an existing test to check that the function entry count is 0 if -sample-profile and -profile-sample-accurate is specified?

Address review comments

Address Teresa's comments, rebase and fix test failures after rebasing.

I have to bump up the index version to 6 after rebasing and fix a few failing test cases.

Rebase and address Teresa's comments.

Sorry for this very long delay in following up.

I also think checking for the change in worklist size better captures the splitting of the SCC than checking C != OldC

Address review comments.

In D48467#1181907, @RKSimon wrote:

@eraman Please can you take a look at rL338358 - this shows how to use getTargetConstantBitsFromNode to avoid a lot of extra checks you need from using getTargetConstantFromNode directly

Updates to work with r338358.

LGTM with a change to the option description .

Change cast_or_null to dyn_cast_or_null

Address Simon's comments.

Address review comments.

Rebase after r336404

Implement this by expanding the patterns generated by isFNEG

I am going to take the approach suggested by Sanjay and expand isFNEG to
handle splat of a negated scalar. Then, the current combineFMA should take
care of the rest. I got the isFNEG to work with shuffle (and work on the
current test case) but haven't yet handled the insertelement case. I am out
traveling for the next ten days and will send a revised patch after I am
back.

Rebase after adding the test case at r335367

No, this doesn't deal with this. Should I start from the negate and push it
down if it is transitively used by fma?

Rebase after r335342

Ping

LGTM

LGTM

This is looking good. Main comment I have is about the number of promotions+inlining that can take place is unbounded. One could argue that if there were many small direct calls, we would inline all of them, but still I feel it is better to limit the number of promotions.

Address Teresa's comments and rebase.

Address review comments.