-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[PGO][Offload] Fix offload coverage mapping #143490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PGO][Offload] Fix offload coverage mapping #143490
Conversation
@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))
|
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
|
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.
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.
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.
This pull request fixes coverage mapping on GPU targets.