MC,AMDGPU: Don't pad .text with s_code_end if it would otherwise be empty #147980
Conversation
We don't want that padding in a module that only contains data, not code.
|
@llvm/pr-subscribers-mc @llvm/pr-subscribers-backend-amdgpu Author: Tim Renouf (trenouf) ChangesWe don't want that padding in a module that only contains data, not code. Full diff: https://github.com/llvm/llvm-project/pull/147980.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
index 749b9efc81378..3765c1d2b106c 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
@@ -486,10 +486,12 @@ bool AMDGPUAsmPrinter::doFinalization(Module &M) {
// Pad with s_code_end to help tools and guard against instruction prefetch
// causing stale data in caches. Arguably this should be done by the linker,
// which is why this isn't done for Mesa.
+ // Don't do it if there are no functions.
const MCSubtargetInfo &STI = *getGlobalSTI();
if ((AMDGPU::isGFX10Plus(STI) || AMDGPU::isGFX90A(STI)) &&
(STI.getTargetTriple().getOS() == Triple::AMDHSA ||
- STI.getTargetTriple().getOS() == Triple::AMDPAL)) {
+ STI.getTargetTriple().getOS() == Triple::AMDPAL) &&
+ !M.empty()) {
OutStreamer->switchSection(getObjFileLowering().getTextSection());
getTargetStreamer()->EmitCodeEnd(STI);
}
|
| (STI.getTargetTriple().getOS() == Triple::AMDHSA || | ||
| STI.getTargetTriple().getOS() == Triple::AMDPAL)) { | ||
| STI.getTargetTriple().getOS() == Triple::AMDPAL) && | ||
| !M.empty()) { |
There was a problem hiding this comment.
Seems reasonable but I wonder if there's a better way to test "did we emit any code?". E.g. what if the module contains some function declarations but no definitions?
There was a problem hiding this comment.
Plus this may misdetect available_externally functions which will be dropped
There was a problem hiding this comment.
I changed to using MCSection::hasInstructions(), which I had to fix to work with the asm streamer.
Also added test. The positive case (when it does add padding) already has a test.
trenouf
changed the title
| @@ -0,0 +1,9 @@ | |||
| ; Test that there is no s_code_end padding if .text is othertwise empty. | |||
There was a problem hiding this comment.
| ; Test that there is no s_code_end padding if .text is othertwise empty. | |
| ; Test that there is no s_code_end padding if .text is otherwise empty. |
We don't want that padding in a module that only contains data, not code.
Also fix MCSection::hasInstructions() so it works with the asm streamer too.