[InstrProf][X86] Set code model for counters global variables #77986
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For the medium/large code models with small/large data splitting, instrumentation-added globals that may vary in size and are assigned an explicit section can cause the large section flag to be inconsistent, usually meaning that some translation units will use 32 bit relocations to reference these globals, but the section will be laid out further from text, causing extra relocation pressure. For the medium code model, mark these globals as "small" since performance may still be important for these globals. For the large code model, mark these globals as "large" to prevent any sort of relocation pressure.
llvmbot
added
PGO
|
@llvm/pr-subscribers-compiler-rt-sanitizer @llvm/pr-subscribers-pgo Author: Arthur Eubanks (aeubanks) ChangesFor the medium/large code models with small/large data splitting, For the medium code model, mark these globals as "small" since performance may still be important for these globals. For the large code model, mark these globals as "large" to prevent any sort of relocation pressure. Full diff: https://github.com/llvm/llvm-project/pull/77986.diff 7 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Instrumentation.h b/llvm/include/llvm/Transforms/Instrumentation.h
index ea97ab2562a5b0..667505ded16a06 100644
--- a/llvm/include/llvm/Transforms/Instrumentation.h
+++ b/llvm/include/llvm/Transforms/Instrumentation.h
@@ -49,10 +49,25 @@ GlobalVariable *createPrivateGlobalForString(Module &M, StringRef Str,
// Returns nullptr on failure.
Comdat *getOrCreateFunctionComdat(Function &F, Triple &T);
-// Place global in a large section for x86-64 ELF binaries to mitigate
-// relocation overflow pressure. This can be be used for metadata globals that
-// aren't directly accessed by code, which has no performance impact.
-void setGlobalVariableLargeSection(const Triple &TargetTriple,
+// Place global in a large section for x86-64 ELF binaries in code models with
+// split small/large data sections to mitigate relocation overflow pressure.
+// This can be be used for metadata globals that aren't directly accessed by
+// code, which has no performance impact.
+void setGlobalVariableLargeCodeModel(const Triple &TargetTriple,
+ GlobalVariable &GV);
+
+// Place global in a small or large section for x86-64 ELF binaries in code
+// models with split small/large data sections. This can be be used for metadata
+// globals that vary in size that are put into an explicit section, which can
+// cause inconsistent small/large section flags on the explicit section. This
+// should only be used for globals that are frequently accessed, otherwise
+// setGlobalVariableLargeCodeModel() should be used to mitigate relocation
+// pressure.
+//
+// Currently this places the variables in a small section for the medium code
+// model, where performance is still a concern, but in a large section for the
+// large code model, where we try to avoid all relocation pressure.
+void setHotGlobalVariableCodeModel(const Triple &TargetTriple,
GlobalVariable &GV);
// Insert GCOV profiling instrumentation
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index fb5838bb7941ad..3ab8a21e340cec 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2181,7 +2181,7 @@ ModuleAddressSanitizer::CreateMetadataGlobal(Module &M, Constant *Initializer,
Metadata->setSection(getGlobalMetadataSection());
// Place metadata in a large section for x86-64 ELF binaries to mitigate
// relocation pressure.
- setGlobalVariableLargeSection(TargetTriple, *Metadata);
+ setGlobalVariableLargeCodeModel(TargetTriple, *Metadata);
return Metadata;
}
diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
index a19b1408725441..1a5b16f4118997 100644
--- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -1285,6 +1285,7 @@ GlobalVariable *InstrLowerer::setupProfileSection(InstrProfInstBase *Inc,
// Put the counters and bitmaps in their own sections so linkers can
// remove unneeded sections.
Ptr->setSection(getInstrProfSectionName(IPSK, TT.getObjectFormat()));
+ setHotGlobalVariableCodeModel(TT, *Ptr);
Ptr->setLinkage(Linkage);
maybeSetComdat(Ptr, Fn, VarName);
return Ptr;
@@ -1450,7 +1451,7 @@ void InstrLowerer::createDataVariable(InstrProfCntrInstBase *Inc) {
M, ValuesTy, false, Linkage, Constant::getNullValue(ValuesTy),
getVarName(Inc, getInstrProfValuesVarPrefix(), Renamed));
ValuesVar->setVisibility(Visibility);
- setGlobalVariableLargeSection(TT, *ValuesVar);
+ setGlobalVariableLargeCodeModel(TT, *ValuesVar);
ValuesVar->setSection(
getInstrProfSectionName(IPSK_vals, TT.getObjectFormat()));
ValuesVar->setAlignment(Align(8));
@@ -1588,7 +1589,7 @@ void InstrLowerer::emitVNodes() {
auto *VNodesVar = new GlobalVariable(
M, VNodesTy, false, GlobalValue::PrivateLinkage,
Constant::getNullValue(VNodesTy), getInstrProfVNodesVarName());
- setGlobalVariableLargeSection(TT, *VNodesVar);
+ setGlobalVariableLargeCodeModel(TT, *VNodesVar);
VNodesVar->setSection(
getInstrProfSectionName(IPSK_vnodes, TT.getObjectFormat()));
VNodesVar->setAlignment(M.getDataLayout().getABITypeAlign(VNodesTy));
@@ -1616,7 +1617,7 @@ void InstrLowerer::emitNameData() {
GlobalValue::PrivateLinkage, NamesVal,
getInstrProfNamesVarName());
NamesSize = CompressedNameStr.size();
- setGlobalVariableLargeSection(TT, *NamesVar);
+ setGlobalVariableLargeCodeModel(TT, *NamesVar);
NamesVar->setSection(
ProfileCorrelate == InstrProfCorrelator::BINARY
? getInstrProfSectionName(IPSK_covname, TT.getObjectFormat())
diff --git a/llvm/lib/Transforms/Instrumentation/Instrumentation.cpp b/llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
index b842d9eef407c2..09fff5f2d79546 100644
--- a/llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
@@ -85,8 +85,8 @@ Comdat *llvm::getOrCreateFunctionComdat(Function &F, Triple &T) {
return C;
}
-void llvm::setGlobalVariableLargeSection(const Triple &TargetTriple,
- GlobalVariable &GV) {
+void llvm::setGlobalVariableLargeCodeModel(const Triple &TargetTriple,
+ GlobalVariable &GV) {
// Limit to x86-64 ELF.
if (TargetTriple.getArch() != Triple::x86_64 ||
TargetTriple.getObjectFormat() != Triple::ELF)
@@ -97,3 +97,19 @@ void llvm::setGlobalVariableLargeSection(const Triple &TargetTriple,
return;
GV.setCodeModel(CodeModel::Large);
}
+
+void llvm::setHotGlobalVariableCodeModel(const Triple &TargetTriple,
+ GlobalVariable &GV) {
+ // Limit to x86-64 ELF.
+ if (TargetTriple.getArch() != Triple::x86_64 ||
+ TargetTriple.getObjectFormat() != Triple::ELF)
+ return;
+ // Limit to medium/large code models.
+ std::optional<CodeModel::Model> CM = GV.getParent()->getCodeModel();
+ if (!CM)
+ return;
+ if (*CM == CodeModel::Medium)
+ GV.setCodeModel(CodeModel::Small);
+ else if (*CM == CodeModel::Large)
+ GV.setCodeModel(CodeModel::Large);
+}
diff --git a/llvm/test/Instrumentation/InstrProfiling/section-code-model-large.ll b/llvm/test/Instrumentation/InstrProfiling/section-code-model-large.ll
index 5ebce67bab86aa..dff86542d26d75 100644
--- a/llvm/test/Instrumentation/InstrProfiling/section-code-model-large.ll
+++ b/llvm/test/Instrumentation/InstrProfiling/section-code-model-large.ll
@@ -24,7 +24,7 @@ attributes #0 = { nounwind }
!0 = !{i32 1, !"Code Model", i32 4}
; CHECK: @__profc_foo =
-; CHECK-NOT: code_model "large"
+; CHECK-SAME: code_model "large"
; CHECK: @__profvp_foo =
; CHECK-SAME: code_model "large"
; CHECK: @__profd_foo =
diff --git a/llvm/test/Instrumentation/InstrProfiling/section-code-model-medium.ll b/llvm/test/Instrumentation/InstrProfiling/section-code-model-medium.ll
index 0b269a87a64448..e575d8e7198b7e 100644
--- a/llvm/test/Instrumentation/InstrProfiling/section-code-model-medium.ll
+++ b/llvm/test/Instrumentation/InstrProfiling/section-code-model-medium.ll
@@ -25,7 +25,8 @@ attributes #0 = { nounwind }
!0 = !{i32 1, !"Code Model", i32 3}
; CHECK: @__profc_foo =
-; CHECK-NOT: code_model "large"
+; X8664-SAME: code_model "small"
+; PPC-NOT: code_model
; CHECK: @__profvp_foo =
; X8664-SAME: code_model "large"
; PPC-NOT: code_model "large"
diff --git a/llvm/test/Instrumentation/InstrProfiling/section-code-model-small.ll b/llvm/test/Instrumentation/InstrProfiling/section-code-model-small.ll
index 11cc1875129518..8c1d595cab3cbe 100644
--- a/llvm/test/Instrumentation/InstrProfiling/section-code-model-small.ll
+++ b/llvm/test/Instrumentation/InstrProfiling/section-code-model-small.ll
@@ -24,7 +24,7 @@ attributes #0 = { nounwind }
!0 = !{i32 1, !"Code Model", i32 1}
; CHECK: @__profc_foo =
-; CHECK-NOT: code_model "large"
+; CHECK-NOT: code_model
; CHECK: @__profvp_foo =
; CHECK-NOT: code_model "large"
; CHECK: @__profd_foo =
|
vitalybuka
approved these changes
Jan 12, 2024
MaskRay
approved these changes
Jan 13, 2024
There was a problem hiding this comment.
IIUC when mixing -mcmodel=small and -mcmodel=large object files, the counter section will cause an lld warning if we take #72335 . This makes me nervous about the lld change.
However, setting the code model for counter variables is right.
|
after talking with @rnk, we're going to go a different route of not applying the large data threshold to globals with explicit sections. that should obsolete this patch and take care of most cases |
aeubanks
added a commit
to aeubanks/llvm-project
that referenced
this pull request
Jan 16, 2024
… section If multiple globals are placed in an explicit section, there's a chance that the large data threshold will cause the different globals to be inconsistent in whether they're large or small. Mixing sections with mismatched large section flags can cause undesirable issues like increased relocation pressure because there may be 32-bit references to the section in some TUs, but the section is considered large since input section flags are unioned and other TUs added the large section flag. An explicit code model on the global still overrides the decision. We can do this for globals without any references to them, like what we did with asan_globals in llvm#74514. If we have some precompiled small code model files where asan_globals is not considered large mixed with medium/large code model files, that's ok because the section is considered large and placed farther. However, overriding the code model for globals in some TUs but not others and having references to them from code will still result in the above undesired behavior. This mitigates a whole class of mismatched large section flag issues like what llvm#77986 was trying to fix. This ends up not adding the SHF_X86_64_LARGE section flag on explicit sections in the medium/large code model. This is ok for the large code model since all references from large text must use 64-bit relocations anyway.
|
abandoning in favor of #78348 |
aeubanks
added a commit
that referenced
this pull request
Jan 17, 2024
… section (#78348) If multiple globals are placed in an explicit section, there's a chance that the large data threshold will cause the different globals to be inconsistent in whether they're large or small. Mixing sections with mismatched large section flags can cause undesirable issues like increased relocation pressure because there may be 32-bit references to the section in some TUs, but the section is considered large since input section flags are unioned and other TUs added the large section flag. An explicit code model on the global still overrides the decision. We can do this for globals without any references to them, like what we did with asan_globals in #74514. If we have some precompiled small code model files where asan_globals is not considered large mixed with medium/large code model files, that's ok because the section is considered large and placed farther. However, overriding the code model for globals in some TUs but not others and having references to them from code will still result in the above undesired behavior. This mitigates a whole class of mismatched large section flag issues like what #77986 was trying to fix. This ends up not adding the SHF_X86_64_LARGE section flag on explicit sections in the medium/large code model. This is ok for the large code model since all references from large text must use 64-bit relocations anyway.
ampandey-1995
pushed a commit
to ampandey-1995/llvm-project
that referenced
this pull request
Jan 19, 2024
… section (llvm#78348) If multiple globals are placed in an explicit section, there's a chance that the large data threshold will cause the different globals to be inconsistent in whether they're large or small. Mixing sections with mismatched large section flags can cause undesirable issues like increased relocation pressure because there may be 32-bit references to the section in some TUs, but the section is considered large since input section flags are unioned and other TUs added the large section flag. An explicit code model on the global still overrides the decision. We can do this for globals without any references to them, like what we did with asan_globals in llvm#74514. If we have some precompiled small code model files where asan_globals is not considered large mixed with medium/large code model files, that's ok because the section is considered large and placed farther. However, overriding the code model for globals in some TUs but not others and having references to them from code will still result in the above undesired behavior. This mitigates a whole class of mismatched large section flag issues like what llvm#77986 was trying to fix. This ends up not adding the SHF_X86_64_LARGE section flag on explicit sections in the medium/large code model. This is ok for the large code model since all references from large text must use 64-bit relocations anyway.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For the medium/large code models with small/large data splitting,
instrumentation-added globals that may vary in size and are assigned
an explicit section can cause the large section flag to be
inconsistent, usually meaning that some translation units will use
32 bit relocations to reference these globals, but the section will be
laid out further from text, causing extra relocation pressure.
For the medium code model, mark these globals as "small" since performance may still be important for these globals.
For the large code model, mark these globals as "large" to prevent any sort of relocation pressure.