[DirectX] Convert private global variables to internal linkage during Finalize Linkage pass

[kmpeng]

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.

[llvmbot]

@llvm/pr-subscribers-backend-directx

Author: Kaitlin Peng (kmpeng)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/146406.diff

1 Files Affected:

• (modified) llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp (+17-3)

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,

[bogner]

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?

[farzonl]

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);

[farzonl]

I'm fine with this change. I beleive Justin's concerns can be addressed when we put up a pr for #139023