[PGO] clang part of change for context-sensitive PGO.

[xur]

Summary

Add new options for context-sensitive PGO.

Refer to https://reviews.llvm.org/D54175

[xur]

update the patch to work with new version in
https://reviews.llvm.org/D54175

[davidxl]

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

Test cases are also missing.

[xur]

Here is the summary of CSPGO options workflow:

(1) CSPGO instrumentation.
We add new driver options of fcs-profile-generate fcs-profile-generate=. They are typically used with -fprofile-use (but not required).
They will not work together with -fprofile-generate (They could, but not do not make sense. There is a hard error for this).

We also add a new instrument kind (csllvm) for cc1 option fprofile-instrument. This is corresponding to a new CodeGen option: CodeGenOptions::ProfileCSIRInstr.

The driver option is parsed and translated to cc1 option in addPGOAndCoverageFlags().

cc1 option is converted to CodeGenOpts in setPGOInstrumentor() (CompilerInvocation.cpp).

CodeGenOpts is passed into llvm pass manager in BackendUtil.cpp:
(1) PMBuilder options for legacy passmanager in EmitAssemblyHelper::CreatePasses().
(2) PGOOptions struct for new passmanager in EmitAssemblyHelper::EmitAssemblyWithNewPassManager().

(2) PGO use
We will set CodeGenOptions::ProfileCSIRInstr in setPGOUseInstrumentor() (CompilerInvocation.cpp) when processing -fprofile-use option.

CodeGenOpts is handled similarly as in CSPGO instrumentation.

(3) lto/thinlto
We need to pass CSPGO instrumentation and PGO profile to the back-end compilation. This is through some new options to -plugin-opt (in tools::AddGoldPlugin()). Note:

• For CSPGO instrumentation, it is guarded by CSPGOGenerateArg.
• For PGO use, I unconditionally add -plugin-opt=cs-profile-path=<> because I don't want to open profile. I could read the profile and check if there is CS counters. But that needs to open the profile file.

The change to lib/CodeGen/CodeGenModule.cpp is for llvm profile summary change.

[xur]

Add a testcase to test the options

[davidxl]

include/clang/Frontend/CodeGenOptions.h

330

The comment is not precise.

lib/CodeGen/BackendUtil.cpp

690

what is going to happen if user uses a merged data with CS profile in profile-use and also specifies CS prof-gen?

948

Can you explain the change at this line?

954

Explain the change at this line.

967

should emit error message instead.

1244

please add +tejohnson for reviewing thinlto related pieces.

lib/Frontend/CompilerInvocation.cpp

504

CSIRInstr and IRInstr are not exclusive to each other, so why use if-then-else here?

[xur]

include/clang/Frontend/CodeGenOptions.h

330

Fixed

lib/CodeGen/BackendUtil.cpp

690

Good question. As of this patch, the behavior is different for the legacy pass manager and the new pass manager.
For the legacy pass manager (the code here), we will call normal PGOUsePass, CSPGOGenPass, and CSPGOUsePass.
For the new pass manager, we will report error of
Assertion `!CodeGenOpts.hasProfileCSIRUse() && "Cannot have both CSProfileUse and CSProfileGen at the same time"' failed.

The behavior for legacy pass manager is wrong. I will fix.

948

My earlier version will have two new booleans (bool RunCSProfileGen and bool RunCSProfileUse).
Here I combined the following 3 field in PGOOptions struct

bool RunProfileGen;
bool RunCSProfileGen;
bool RunCSProfileUse;

into an enum. There three are mutually exclusive.

lib/Frontend/CompilerInvocation.cpp

504

For CodeGenOptions kind, CSIRInstr and IRinstr are exclusive -- there is just one enum for the kind.

[xur]

Integrated David's review comments. Also add a test to test the error when both CSInstrumentGen and CSInstrumentUse on.

[tejohnson]

lib/CodeGen/BackendUtil.cpp

1244

This part looks fine (haven't examined the rest in detail). Please add a test. There are some existing ones for the ThinLTO backend via clang in test/CodeGen/thinlto-* that you can look at for examples.

[xur]

Added a thinlto test case and fixed a few issues.

[xur]

Updated the patch to sync with the updates in https://reviews.llvm.org/D54175

[lebedev.ri]

Clang reviews should mainly go to cfe-commits, not llvm-commits. (all that will happen automagically if one sets the correct repo for the differential..)

[xur]

Update the patch to sync with https://reviews.llvm.org/D54175

[xur]

Updated the patch to sync with D54175

[tejohnson]

LGTM except for place noted below where I disagree with a change made earlier. Will let @davidxl chime in if he disagrees with me or has any other comments.

lib/CodeGen/BackendUtil.cpp

967

I disagree with David's comment here. All the error checking should (and is) being done in Clang.cpp where the options are being processed. These and the places above for the old PM should just be asserts.

[davidxl]

Please add changes to option documentation.

[xur]

Integrated Teresa's suggestion to change the err() to assert.
Added documentation change suggested by David.

[davidxl]

docs/UsersManual.rst

1792 ↗

(On Diff #188955)

Please add a use example to show the typical flow with this option (both gen and use phases -- including llvm-profdata step).

[xur]

Added usage example to UserManual.rst suggested by David.

[davidxl]

lgtm