[StaticDataLayout][MemProf] Record whether the IR is compiled with data access profiles by module flag. #162333
Conversation
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-pgo Author: Mingming Liu (mingmingl-llvm) ChangesThe codegen pass in the pipeline can read the module flag to tell whether the IR is compiled with data access profile, to support two use cases when
#155337 shows the motivating use case in function Full diff: https://github.com/llvm/llvm-project/pull/162333.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp
index c86092bd51eda..d86fcf268ce4f 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp
@@ -798,6 +798,7 @@ bool MemProfUsePass::annotateGlobalVariables(
return false;
if (!DataAccessProf) {
+ M.addModuleFlag(Module::Warning, "EnableDataAccessProf", 0U);
M.getContext().diagnose(DiagnosticInfoPGOProfile(
MemoryProfileFileName.data(),
StringRef("Data access profiles not found in memprof. Ignore "
@@ -805,6 +806,7 @@ bool MemProfUsePass::annotateGlobalVariables(
DS_Warning));
return false;
}
+ M.addModuleFlag(Module::Warning, "EnableDataAccessProf", 1U);
bool Changed = false;
// Iterate all global variables in the module and annotate them based on
diff --git a/llvm/test/Transforms/PGOProfile/data-access-profile.ll b/llvm/test/Transforms/PGOProfile/data-access-profile.ll
index 29198f34ccbba..d9976a9d23755 100644
--- a/llvm/test/Transforms/PGOProfile/data-access-profile.ll
+++ b/llvm/test/Transforms/PGOProfile/data-access-profile.ll
@@ -3,22 +3,28 @@
; RUN: rm -rf %t && split-file %s %t && cd %t
-;; Read a text profile and merge it into indexed profile.
+;; Read text profiles and merge them into indexed profiles.
; RUN: llvm-profdata merge --memprof-version=4 memprof.yaml -o memprof.profdata
+; RUN: llvm-profdata merge --memprof-version=4 memprof-no-dap.yaml -o memprof-no-dap.profdata
;; Run optimizer pass on an IR module without IR functions, and test that global
;; variables in the module could be annotated (i.e., no early return),
; RUN: opt -passes='memprof-use<profile-filename=memprof.profdata>' -memprof-annotate-static-data-prefix \
-; RUN: -debug-only=memprof -stats -S funcless-module.ll -o - 2>&1 | FileCheck %s --check-prefixes=LOG,PREFIX,STAT
+; RUN: -debug-only=memprof -stats -S funcless-module.ll -o - 2>&1 | FileCheck %s --check-prefixes=LOG,IR,STAT
;; Run optimizer pass on the IR, and check the section prefix.
; RUN: opt -passes='memprof-use<profile-filename=memprof.profdata>' -memprof-annotate-static-data-prefix \
-; RUN: -debug-only=memprof -stats -S input.ll -o - 2>&1 | FileCheck %s --check-prefixes=LOG,PREFIX,STAT
+; RUN: -debug-only=memprof -stats -S input.ll -o - 2>&1 | FileCheck %s --check-prefixes=LOG,IR,STAT
-;; Run optimizer pass without explicitly setting -memprof-annotate-static-data-prefix.
-;; The output text IR shouldn't have `section_prefix`
+;; Run memprof without providing memprof data. Test that IR has module flag
+;; `EnableDataAccessProf` as 0.
+; RUN: opt -passes='memprof-use<profile-filename=memprof-no-dap.profdata>' -memprof-annotate-static-data-prefix \
+; RUN: -debug-only=memprof -stats -S input.ll -o - 2>&1 | FileCheck %s --check-prefix=FLAG
+
+;; Run memprof without explicitly setting -memprof-annotate-static-data-prefix.
+;; The output text IR shouldn't have `section_prefix` or EnableDataAccessProf module flag.
; RUN: opt -passes='memprof-use<profile-filename=memprof.profdata>' \
-; RUN: -debug-only=memprof -stats -S input.ll -o - | FileCheck %s --implicit-check-not="section_prefix"
+; RUN: -debug-only=memprof -stats -S input.ll -o - | FileCheck %s --check-prefix=FLAGLESS --implicit-check-not="section_prefix"
; LOG: Skip annotating string literal .str
; LOG: Global variable var1 is annotated as hot
@@ -29,29 +35,33 @@
; LOG: Global variable var4 has explicit section name. Skip annotating.
;; String literals are not annotated.
-; PREFIX: @.str = unnamed_addr constant [5 x i8] c"abcde"
-; PREFIX-NOT: section_prefix
-; PREFIX: @var1 = global i32 123, !section_prefix !0
+; IR: @.str = unnamed_addr constant [5 x i8] c"abcde"
+; IR-NOT: section_prefix
+; IR: @var1 = global i32 123, !section_prefix !0
;; @var.llvm.125 will be canonicalized to @var2 for profile look-up.
-; PREFIX-NEXT: @var2.llvm.125 = global i64 0, !section_prefix !0
+; IR-NEXT: @var2.llvm.125 = global i64 0, !section_prefix !0
;; @bar is not seen in hot symbol or known symbol set, so it won't get a section
;; prefix. Test this by testing that there is no section_prefix between @bar and
;; @foo.
-; PREFIX-NEXT: @bar = global i16 3
-; PREFIX-NOT: !section_prefix
+; IR-NEXT: @bar = global i16 3
+; IR-NOT: !section_prefix
;; @foo is unlikely.
-; PREFIX-NEXT: @foo = global i8 2, !section_prefix !1
+; IR-NEXT: @foo = global i8 2, !section_prefix !1
+
+; IR-NEXT: @var3 = constant [2 x i32] [i32 12345, i32 6789], section "sec1"
+; IR-NEXT: @var4 = constant [1 x i64] [i64 98765] #0
-; PREFIX-NEXT: @var3 = constant [2 x i32] [i32 12345, i32 6789], section "sec1"
-; PREFIX-NEXT: @var4 = constant [1 x i64] [i64 98765] #0
+; IR: attributes #0 = { "rodata-section"="sec2" }
-; PREFIX: attributes #0 = { "rodata-section"="sec2" }
+; IR: !0 = !{!"section_prefix", !"hot"}
+; IR-NEXT: !1 = !{!"section_prefix", !"unlikely"}
+; IR-NEXT: !2 = !{i32 2, !"EnableDataAccessProf", i32 1}
-; PREFIX: !0 = !{!"section_prefix", !"hot"}
-; PREFIX-NEXT: !1 = !{!"section_prefix", !"unlikely"}
+; FLAG: !{i32 2, !"EnableDataAccessProf", i32 0}
+; FLAGLESS-NOT: EnableDataAccessProf
; STAT: 1 memprof - Number of global vars annotated with 'unlikely' section prefix.
; STAT: 2 memprof - Number of global vars with user-specified section (not annotated).
@@ -72,6 +82,24 @@ DataAccessProfiles:
- foo
KnownColdStrHashes: [ 999, 1001 ]
...
+;--- memprof-no-dap.yaml
+---
+# A memprof file with without data access profiles. The heap records are simplified
+# to pass profile parsing and don't need to match the IR.
+HeapProfileRecords:
+ - GUID: 0xdeadbeef12345678
+ AllocSites:
+ - Callstack:
+ - { Function: 0x1111111111111111, LineOffset: 11, Column: 10, IsInlineFrame: true }
+ MemInfoBlock:
+ AllocCount: 111
+ TotalSize: 222
+ TotalLifetime: 333
+ TotalLifetimeAccessDensity: 444
+ CallSites:
+ - Frames:
+ - { Function: 0x5555555555555555, LineOffset: 55, Column: 50, IsInlineFrame: true }
+...
;--- input.ll
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
|
| return false; | ||
|
|
||
| if (!DataAccessProf) { | ||
| M.addModuleFlag(Module::Warning, "EnableDataAccessProf", 0U); |
There was a problem hiding this comment.
What if the function doesn't have any variables that get data access profiles? I assume we have an non-missing but empty DataAccessProfiles section so we wouldn't come in here?
There was a problem hiding this comment.
What if the function doesn't have any variables that get data access profiles? I assume we have an non-missing but empty DataAccessProfiles section so we wouldn't come in here?
The data access profile payload is generated per binary and parsed as a whole in an optimizer pass. There are no selective parsing logic that filters the payload based on the module IR.
So there are basically three cases
- If
memprof-annotate-static-data-prefixis false or if the module doesn't have global variables, the module won't have module flag (due to early return at line 797) - If 1) is not met and the payload is empty in the memprof file, the condition at line 800 is true and module flag is zero.
- if 2) is not met (i.e., payload is not empty), the module will have module flag as one.
On the module flag user side, it regards case 3 as 'the module is compiled with data access profile' and treats empty section prefix as unknown, and regards the other two cases as 'the module isn't compiled with data access profiles' and override empty section prefix if needed.
I wonder if it's clearer to combine 1) and 2) so the module flag is always present as long as this pass runs.
There was a problem hiding this comment.
Thanks for the explanation, this makes sense.
I wonder if it's clearer to combine 1) and 2) so the module flag is always present as long as this pass runs.
I don't think you want to do that, because a module without any globals would get a flag value of 0, and per Module::Warning semantics, if it is combined with another module via an LTO link the first value would win. I think you want this to emit the same value, or none, for all modules using the same profile.
| return false; | ||
|
|
||
| if (!DataAccessProf) { | ||
| M.addModuleFlag(Module::Warning, "EnableDataAccessProf", 0U); |
There was a problem hiding this comment.
Thanks for the explanation, this makes sense.
I wonder if it's clearer to combine 1) and 2) so the module flag is always present as long as this pass runs.
I don't think you want to do that, because a module without any globals would get a flag value of 0, and per Module::Warning semantics, if it is combined with another module via an LTO link the first value would win. I think you want this to emit the same value, or none, for all modules using the same profile.
yeah this makes sense. Actually the module flag semantic is chosen to be warning to accommodate for the un-common case when one translation unit doesn't have this module flag (e.g., use per translation unit build flags to work around build issues); we don't want the whole build to fail due to different module flag values. |
…ta access profiles by module flag. (llvm#162333) The codegen pass in the pipeline can read the module flag to tell whether the IR is compiled with data access profile, to support two use cases when `memprof-annotate-static-data-prefix=true` is enabled 1. The binary is compiled with data access profiles. - The module flag will have value 1, and codegen pass should regard an empty section prefix as 'unknown' and conservatively not placing the data into `.unlikely` data sections. 2. The binary is compiled without data access profiles (e.g., during incremental rollout, etc) - The module flag will have value 0, and codegen pass can override an empty section prefix based on PGO counters. llvm#155337 shows the motivating use case in function `StaticDataProfileInfo::getConstantSectionPrefix` in `llvm/lib/Analysis/StaticDataProfileInfo.cpp` This is the 1st patch as a split of llvm#155337
3 participants
The codegen pass in the pipeline can read the module flag to tell whether the IR is compiled with data access profile, to support two use cases when
memprof-annotate-static-data-prefix=trueis enabledThe binary is compiled with data access profiles.
.unlikelydata sections.The binary is compiled without data access profiles (e.g., during incremental rollout, etc)
#155337 shows the motivating use case in function
StaticDataProfileInfo::getConstantSectionPrefixinllvm/lib/Analysis/StaticDataProfileInfo.cppThis is the 1st patch as a split of #155337