[DirectX] Convert private global variables to internal linkage during Finalize Linkage pass #146406
Conversation
|
@llvm/pr-subscribers-backend-directx Author: Kaitlin Peng (kmpeng) ChangesFixes #140420. The switch.table.* validation errors were caused by DXIL not supporting private global variables. Converting them to internal linkage fixes the bug. May need more discussion on the preserved analyses/a follow-up PR that fixes what this pass says it preserves. Full diff: https://github.com/llvm/llvm-project/pull/146406.diff 1 Files Affected:
diff --git a/llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp b/llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp
index 94b2dbe78c4f7..5f331dbd2d826 100644
--- a/llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp
+++ b/llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp
@@ -18,6 +18,16 @@
using namespace llvm;
static bool finalizeLinkage(Module &M) {
+ bool MadeChange = false;
+
+ // Convert private global variables to internal linkage.
+ for (GlobalVariable &GV : M.globals()) {
+ if (GV.hasPrivateLinkage()) {
+ GV.setLinkage(GlobalValue::InternalLinkage);
+ MadeChange = true;
+ }
+ }
+
SmallVector<Function *> Funcs;
// Collect non-entry and non-exported functions to set to internal linkage.
@@ -32,13 +42,17 @@ static bool finalizeLinkage(Module &M) {
}
for (Function *F : Funcs) {
- if (F->getLinkage() == GlobalValue::ExternalLinkage)
+ if (F->getLinkage() == GlobalValue::ExternalLinkage) {
F->setLinkage(GlobalValue::InternalLinkage);
- if (F->isDefTriviallyDead())
+ MadeChange = true;
+ }
+ if (F->isDefTriviallyDead()) {
M.getFunctionList().erase(F);
+ MadeChange = true;
+ }
}
- return false;
+ return MadeChange;
}
PreservedAnalyses DXILFinalizeLinkage::run(Module &M,
|
| if (GV.hasPrivateLinkage()) { | ||
| GV.setLinkage(GlobalValue::InternalLinkage); | ||
| MadeChange = true; | ||
| } |
There was a problem hiding this comment.
Should we be removing these if GV.use_empty() to match what we do with functions? Alternatively, if we don't expect to ever get here with unused private globals, can we assert(!GV.use_empty()) here? Or is it okay/expected for us to leave unused internal variables around?
There was a problem hiding this comment.
Thats the solution we worked for this ticket #139023. My understanding was though we didn't want to blanket do this because of external linkage. so, I suppose we could do something like
if(GV.use_empty() && !GV.hasExtenalLinkage())
ToErase.push_back(&G);
Fixes #140420. The switch.table.* validation errors were caused by DXIL not supporting private global variables. Converting them to internal linkage fixes the bug.
May need more discussion on the preserved analyses/a follow-up PR that fixes what this pass says it preserves.