Chandler's example does not involve any indirect calls so it is not related to this patch.

lgtm

lgtm

Added another debug option.

lgtm

Addressed review comments.

Added one more test case to show the merged live-in values from multiple predecessors in the loop.

lgtm ( The area of linkage handling has many subtle problems in the past with large C++ apps, so be prepared for any breakage introduced).

lgtm

Addressed Vedant's comments.

Regarding turning this on for FE instrumentation, it can be done, but probably as a follow up (which also needs loop canonlicalization and rotation). More coverage test performance data (possibly at O0) also need to be collected.

Any update on this patch?

lgtm

Add a test case.

See the summary for precision data. With this patch, the multi-threaded counter value is much closer to the single threaded case with very little loss.

That looks like it - it is in the indirectbr clause, so it might be a good idea to state something in the section describing 'blockaddress' ?

The spec probably needs update, but I remember inliner or function cloning does something to avoid this. +cc Chandler for comments.

This is assuming the referencing the saved addresses is only within the extracted function, but reference to block addresses from outside the parent function is illegal. How do you prevent the original function from branch into the middle of the outlined function?

It is probably not legal to outline a BB with BB's address captured, so your previous version is better.

Any more comments?

So the profile is from GCC built binary ?

Does it miss opportunities to profile annotate inline instance of indirect target?

Addressed Davide's comments.

Simplify test cases.

lgtm

Do you have performance numbers (to show it works better on average)?

Addressed Wei's feedback.

Fix bugs in tests.

thanks for the review! I am pretty happy with the final version .

Address review feebacks.

this looks fine.

Added support to handle integer with any bit-width. New test cases are added to test int type other than i1.

Use all_of idiom.

Address Wei's feedback. Find alloca candidates from Entry blocks .

To be clear, the case handled here is
void foo()
{

...
{
     A a;
     ...
 }

}

yes, those cases can be and will be handled but as a follow up. This patch handles cases where locals are declared in the outlined region.

addressed Davide's feedback.

Address Chandler's feedbacks.

The code will be simplified (with related code removed) once the new PM is the default, but I don't see harm to change this way. lgtm

It is basically a choice between a layout (exiting) that has 50% chance of not taking any branches , 25% of taking one branch, and 25% of taking more than one branches vs the new layout that has only 6.25% chance of taking zero branch and 93.75% of taking only one branch.

Looking at the example. Comparing two layout decisions: 1) always pick the next test as layout successor ; and 2) always pick the (else) if-then block as the layout successor.
A) the number of taken branches is the same
B) assuming 50% prob for all branches, the dynamic count of taken branches with 2) is actually smaller.

I was aware that partial inlining is available in both pre and post link thinLTO pipelines. It is just not tuned. In fact, the right thing to do for ThinLTO is only do function outlining in the prelink step -- this will shrink function size and exposes more importing. The regular inliner in the post -link phase can handle those functions properly. Post thin-link needs another round of partial inlining after the inliner.

For partial inlining, leave it consistent with old PM for now. We can introduce later an internal option to enable/disable pre-thin-link partial inlining and collect more performance number.

lgtm

Any more comments?

added internal option for new PM as well.

Addressed review comments.

Any more comments?

Address review feedback.

Address Chandler's feedbacks.