Page MenuHomePhabricator

[IR] Add hot to function attributes and use hot/cold attribute in function section prefix/suffix
ClosedPublic

Authored by xur on Dec 2 2020, 9:46 AM.

Details

Summary

Clang FE currently has hot/cold function attribute. But we only have cold
function attribute in LLVM IR.

This patch adds support of hot function attribute to LLVM IR.
This attribute will be used in setting function section prefix/suffix.
Currently .hot and .unlikely suffix only are added in PGO (Sample PGO)
compilation (through isFunctionHotInCallGraph and isFunctionColdInCallGraph).

This patch changes the behavior. The new behavior is:
(1) If the user annotates a function as hot or isFunctionHotInCallGraph
is true, this function will be marked as hot.
(2) If the user annotates a function as cold or isFunctionColdInCallGraph
is true, this function will be marked as cold.

The changes are:
(1) user annotated function will used in setting function section prefix/suffix.
(2) hot attribute overwrites profile count based hotness.
(3) profile count based hotness overwrite user annotated cold attribute.

The intention for these changes is to provide the user a way to mark
certain function as hot in cases where training input is hard to cover all
the hot functions.

One thing that we might also need to change is we set cold attribute
in PGO instrumentation. This code is before isFunctionColdInCallGraph and
not use PSI. With this patch, we may mark more cold functions.
I did not change this part because cold attribute is used many downstream
optimizations other than setting function section prefix/suffix.

Diff Detail

Event Timeline

xur created this revision.Dec 2 2020, 9:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2020, 9:46 AM
xur requested review of this revision.Dec 2 2020, 9:46 AM
xur retitled this revision from IR] Add hot to function attributes and use hot/cold attribute in function section prefix/suffix to [IR] Add hot to function attributes and use hot/cold attribute in function section prefix/suffix.Dec 2 2020, 10:06 AM

For documentation, please update docs/LangRef.rst.

Since this is a LangRef change, I suggest sending the summary of the patch as a RFC to llvm-dev list first to hear opinions from the community first.

arsenm added a subscriber: arsenm.Dec 3 2020, 6:25 PM

This is missing all of the asm parser/printer and bitcode tests

xur updated this revision to Diff 311731.Dec 14 2020, 3:42 PM

Updated LangRef and added test cases per review comments.

davidxl added inline comments.Dec 16 2020, 10:14 AM
llvm/docs/LangRef.rst
1505

typo: information.

llvm/lib/CodeGen/CodeGenPrepare.cpp
476

document that this is a conservative behavior.

481–485

For conservative behavior, this should be &&

llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
1926

Perhaps emit a warning here?

MaskRay added inline comments.
clang/lib/CodeGen/CodeGenModule.cpp
1747–1748
clang/test/CodeGen/attributes.c
66

Nit: use [[#HOTDEF:]] (which is shorthand for [[#%u,HOTDEF:]]

xur marked 4 inline comments as done.Dec 16 2020, 10:37 AM
xur added inline comments.
clang/test/CodeGen/attributes.c
66

Thanks. Good to know this format.

llvm/docs/LangRef.rst
1505

Fixed

llvm/lib/CodeGen/CodeGenPrepare.cpp
481–485

Should not be && -- this would require all cold functions be marked as cold by the users to place into unlikely section. I think it's too much.

I used '||" for the following reasons:
(1) If a function is not annotated as "cold", use PSI to determine if to place into unlikely section. This is no change from current behavior.
(2) If the function is marked as code
(2.1) if PSI shows the function is HOT, place into 'hot' section
(2.2) if PSI shows cold, place into 'unlikely' section
(2.3) if PSI shows neither code nor hot. This is a warm function. We honor the annotation, and place into 'unlikely" section.

llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
1926

Added.

xur updated this revision to Diff 312273.Dec 16 2020, 11:31 AM
xur marked 3 inline comments as done.

Integrated comments from David and MaskRay.

davidxl accepted this revision.Dec 17 2020, 1:02 PM

lgtm

This revision is now accepted and ready to land.Dec 17 2020, 1:02 PM
xur added inline comments.Dec 17 2020, 5:23 PM
clang/test/CodeGen/attributes.c
66

@MaskRay: This does not work for me. #HOTDEF only matches numbers. I put ##HOTDEF here and add # to the uses later. But the match still failed.

I reverted to old format (also match the existing code. I plan to commit as it is. Feel free to simplify this after my commit. Thanks!

Herald added a project: Restricted Project. · View Herald TranscriptDec 17 2020, 6:55 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript