This is not completely NFC as it modifies branch weight update behavior (though correctly), so please do more pgo testings to avoid surprises (e.g, some weird dependencies on old behavior).

Looked at the Instruction::updateProfWeight() --- the part that update branch_weights seems bogus -- there is need need to scale branch weight at all.

Can you clarify? IIRC, we never update the per-call site profile meta data with inline transformations.

It is possible to for an entry with zero count -- as for instance in instrumentation PGO. Should it be fixed in place where the div by zero happens?

lgtm

lgtm

Look fine in general. Easwaran can help with review of pass pipeline changes

Missing unit tests for the feature.

lgtm

lgtm

The secondary map 'ProfileData' is needed occasionally (to use cfg hash). doing sorting with std::map is not needed strictly speaking, but it has the nice property of having fixed order.

The number of functions with name conflicts (and requires) using secondary map is very few, so the performance should not be affected.

lgtm

thanks. Can you add a few test cases in the patch?

Please add changes to option documentation.

lgtm

lgtm

lgtm

lgtm

lgtm

LLVM part looks ok. Do you have plan to hook it up with Clang driver? It is also good to add support in llvm-profdata to support converting the raw mapping+dumping into linker order file format.

lgtm

lgtm.

lgtm

lgtm

Please also add a profile dumping/reading round trip test with isCS flag (there are existing similar test for llvm-profdata)

There is a copy of InstrProfData.inc in compiler-rt, and that one needs to be kept in sync with the one in compiler side.

Have not looked at it in details, but it looks like the support for the new PM is missing. Can that be added?

Thanks for the data. It is certainly a great improvement over the hybrid WPD.

Do you have any benchmarking numbers?

Vtable funcs info should be in the initializers of vtable global variables right?

A very high level meta question: can thinLink phase read symbol table from each module? If yes, why duplicate the symtab information into summary?

lgtm

lgtm

why is list-below-cutoff option useful?

lgtm

lgtm

lgtm

Can you write a test case with a shared library that is instrumented and write a main program with dlopen to demonstrate the problem if the fix is not there?

Test case?

lgtm

Can you add a summary of this change, i.e., the driver option processing flow for csPGO.

There are more rooms to improve, but can be done independently.

lgtm

lgtm

lgtm too.

lgtm

lgtm

Can you add a simple test case making sure the func entry is properly annotated?

Test cases are missing in the patch.

lgtm