[PGO][Offload] Fix offload coverage mapping #143490
Merged
Conversation
This file contains hidden or 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
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-offload Author: Ethan Luis McDonough (EthanLuisMcDonough) ChangesThis pull request fixes coverage mapping on GPU targets.
Full diff: https://github.com/llvm/llvm-project/pull/143490.diff 5 Files Affected:
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 73811d15979d5..080de82081aad 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -2622,8 +2622,9 @@ void CoverageMappingModuleGen::emit() {
CGM.addUsedGlobal(CovData);
// Create the deferred function records array
if (!FunctionNames.empty()) {
- auto NamesArrTy = llvm::ArrayType::get(llvm::PointerType::getUnqual(Ctx),
- FunctionNames.size());
+ auto AddrSpace = FunctionNames.front()->getType()->getPointerAddressSpace();
+ auto NamesArrTy = llvm::ArrayType::get(
+ llvm::PointerType::get(Ctx, AddrSpace), FunctionNames.size());
auto NamesArrVal = llvm::ConstantArray::get(NamesArrTy, FunctionNames);
// This variable will *NOT* be emitted to the object file. It is used
// to pass the list of names referenced to codegen.
diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
index 034678527950d..8dce227392508 100644
--- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -1954,12 +1954,6 @@ void InstrLowerer::emitNameData() {
GlobalValue::PrivateLinkage, NamesVal,
getInstrProfNamesVarName());
- // Make names variable public if current target is a GPU
- if (isGPUProfTarget(M)) {
- NamesVar->setLinkage(GlobalValue::ExternalLinkage);
- NamesVar->setVisibility(GlobalValue::VisibilityTypes::ProtectedVisibility);
- }
-
NamesSize = CompressedNameStr.size();
setGlobalVariableLargeSection(TT, *NamesVar);
NamesVar->setSection(
diff --git a/offload/plugins-nextgen/common/include/GlobalHandler.h b/offload/plugins-nextgen/common/include/GlobalHandler.h
index 6def53430a7c0..5d6109df49da5 100644
--- a/offload/plugins-nextgen/common/include/GlobalHandler.h
+++ b/offload/plugins-nextgen/common/include/GlobalHandler.h
@@ -80,6 +80,7 @@ struct GPUProfGlobals {
void dump() const;
Error write() const;
+ bool empty() const;
};
/// Subclass of GlobalTy that holds the memory for a global of \p Ty.
@@ -192,9 +193,6 @@ class GenericGlobalHandlerTy {
/*D2H=*/false);
}
- /// Checks whether a given image contains profiling globals.
- bool hasProfilingGlobals(GenericDeviceTy &Device, DeviceImageTy &Image);
-
/// Reads profiling data from a GPU image to supplied profdata struct.
/// Iterates through the image symbol table and stores global values
/// with profiling prefixes.
diff --git a/offload/plugins-nextgen/common/src/GlobalHandler.cpp b/offload/plugins-nextgen/common/src/GlobalHandler.cpp
index 35a70d8eff901..bf32b8ae8569a 100644
--- a/offload/plugins-nextgen/common/src/GlobalHandler.cpp
+++ b/offload/plugins-nextgen/common/src/GlobalHandler.cpp
@@ -164,16 +164,6 @@ Error GenericGlobalHandlerTy::readGlobalFromImage(GenericDeviceTy &Device,
return Plugin::success();
}
-bool GenericGlobalHandlerTy::hasProfilingGlobals(GenericDeviceTy &Device,
- DeviceImageTy &Image) {
- GlobalTy global(getInstrProfNamesVarName().str(), 0);
- if (auto Err = getGlobalMetadataFromImage(Device, Image, global)) {
- consumeError(std::move(Err));
- return false;
- }
- return true;
-}
-
Expected<GPUProfGlobals>
GenericGlobalHandlerTy::readProfilingGlobals(GenericDeviceTy &Device,
DeviceImageTy &Image) {
@@ -195,12 +185,17 @@ GenericGlobalHandlerTy::readProfilingGlobals(GenericDeviceTy &Device,
// Check if given current global is a profiling global based
// on name
if (*NameOrErr == getInstrProfNamesVarName()) {
- // Read in profiled function names
- DeviceProfileData.NamesData = SmallVector<uint8_t>(Sym.getSize(), 0);
- GlobalTy NamesGlobal(NameOrErr->str(), Sym.getSize(),
- DeviceProfileData.NamesData.data());
- if (auto Err = readGlobalFromDevice(Device, Image, NamesGlobal))
- return Err;
+ // Read in profiled function names from ELF
+ auto SectionOrErr = Sym.getSection();
+ if (!SectionOrErr)
+ return SectionOrErr.takeError();
+
+ auto ContentsOrErr = (*SectionOrErr)->getContents();
+ if (!ContentsOrErr)
+ return ContentsOrErr.takeError();
+
+ SmallVector<uint8_t> NameBytes(ContentsOrErr->bytes());
+ DeviceProfileData.NamesData = NameBytes;
} else if (NameOrErr->starts_with(getInstrProfCountersVarPrefix())) {
// Read global variable profiling counts
SmallVector<int64_t> Counts(Sym.getSize() / sizeof(int64_t), 0);
@@ -311,3 +306,7 @@ Error GPUProfGlobals::write() const {
return Plugin::success();
}
+
+bool GPUProfGlobals::empty() const {
+ return Counts.empty() && Data.empty() && NamesData.empty();
+}
diff --git a/offload/plugins-nextgen/common/src/PluginInterface.cpp b/offload/plugins-nextgen/common/src/PluginInterface.cpp
index 059f14f59c38b..db76d4b7a1faa 100644
--- a/offload/plugins-nextgen/common/src/PluginInterface.cpp
+++ b/offload/plugins-nextgen/common/src/PluginInterface.cpp
@@ -854,14 +854,13 @@ Error GenericDeviceTy::deinit(GenericPluginTy &Plugin) {
for (auto *Image : LoadedImages) {
GenericGlobalHandlerTy &Handler = Plugin.getGlobalHandler();
- if (!Handler.hasProfilingGlobals(*this, *Image))
- continue;
-
- GPUProfGlobals profdata;
auto ProfOrErr = Handler.readProfilingGlobals(*this, *Image);
if (!ProfOrErr)
return ProfOrErr.takeError();
+ if (ProfOrErr->empty())
+ continue;
+
// Dump out profdata
if ((OMPX_DebugKind.get() & uint32_t(DeviceDebugKind::PGODump)) ==
uint32_t(DeviceDebugKind::PGODump))
|
jhuber6
approved these changes
Jun 10, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/24284 Here is the relevant piece of the build log for the reference |
tomtor
pushed a commit
to tomtor/llvm-project
that referenced
this pull request
Jun 14, 2025
This pull request fixes coverage mapping on GPU targets. - It adds an address space cast to the coverage mapping generation pass. - It reads the profiled function names from the ELF directly. Reading it from public globals was causing issues in cases where multiple device-code object files are linked together.
akuhlens
pushed a commit
to akuhlens/llvm-project
that referenced
this pull request
Jun 24, 2025
This pull request fixes coverage mapping on GPU targets. - It adds an address space cast to the coverage mapping generation pass. - It reads the profiled function names from the ELF directly. Reading it from public globals was causing issues in cases where multiple device-code object files are linked together.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This pull request fixes coverage mapping on GPU targets.