[llvm] Fix SPIR-V function ordering violation in linker (PR #145039)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lto

Author: Paulius Velesko (pvelesko)

<details>
<summary>Changes</summary>

SPIR-V specification requires all function declarations to appear before
any function definitions. However, LLVM's linker was producing modules
where declarations appeared after definitions, causing SPIR-V validation
failures and compilation errors.

Problem:
- Function declarations (declare) were scattered throughout the module
- Some declarations appeared after function definitions (define)
- This violates SPIR-V spec section 2.4 which mandates declaration-before-definition ordering
- Resulted in invalid SPIR-V modules that failed validation

Solution:
- Added SPIR-V target detection in ModuleLinker::run()
- Implemented reorderFunctionsForSPIRV() to enforce proper ordering
- Only activates for SPIR-V targets (spirv, spirv32, spirv64)
- Reorders functions: all declarations first, then all definitions
- Preserves function references and module integrity

The fix ensures SPIR-V modules comply with the specification while
maintaining zero overhead for non-SPIR-V targets.

Fixes linking errors for HIP/OpenCL programs targeting SPIR-V.

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


1 Files Affected:

- (modified) llvm/lib/Linker/LinkModules.cpp (+70) 


``````````diff
diff --git a/llvm/lib/Linker/LinkModules.cpp b/llvm/lib/Linker/LinkModules.cpp
index 485ac106d4ebb..dd0ac51f22cef 100644
--- a/llvm/lib/Linker/LinkModules.cpp
+++ b/llvm/lib/Linker/LinkModules.cpp
@@ -19,6 +19,7 @@
 #include "llvm/IR/Module.h"
 #include "llvm/Linker/Linker.h"
 #include "llvm/Support/Error.h"
+#include "llvm/TargetParser/Triple.h"
 using namespace llvm;
 
 namespace {
@@ -105,6 +106,10 @@ class ModuleLinker {
                           const DenseSet<const Comdat *> &ReplacedDstComdats);
 
   bool linkIfNeeded(GlobalValue &GV, SmallVectorImpl<GlobalValue *> &GVToClone);
+  
+  /// Reorder functions in the module to ensure SPIR-V compliance.
+  /// SPIR-V requires all function declarations to appear before any function definitions.
+  void reorderFunctionsForSPIRV(Module &M);
 
 public:
   ModuleLinker(IRMover &Mover, std::unique_ptr<Module> SrcM, unsigned Flags,
@@ -615,9 +620,74 @@ bool ModuleLinker::run() {
   if (InternalizeCallback)
     InternalizeCallback(DstM, Internalize);
 
+  // For SPIR-V targets, ensure proper function ordering to comply with SPIR-V specification
+  // SPIR-V requires all function declarations to appear before any function definitions
+  Triple TargetTriple(DstM.getTargetTriple());
+  if (TargetTriple.isSPIRV()) {
+    reorderFunctionsForSPIRV(DstM);
+  }
+
   return false;
 }
 
+void ModuleLinker::reorderFunctionsForSPIRV(Module &M) {
+  // Collect all function declarations and definitions
+  std::vector<Function*> declarations;
+  std::vector<Function*> definitions;
+  
+  // Check if reordering is needed by detecting if any declarations appear after definitions
+  bool needsReordering = false;
+  bool seenDefinition = false;
+  
+  for (Function &F : M) {
+    if (F.isDeclaration()) {
+      declarations.push_back(&F);
+      if (seenDefinition) {
+        needsReordering = true;
+      }
+    } else {
+      definitions.push_back(&F);
+      seenDefinition = true;
+    }
+  }
+  
+  // If no reordering is needed, return early
+  if (!needsReordering) {
+    return;
+  }
+  
+  // Handle global arrays that reference functions (@llvm.used, @llvm.compiler.used)
+  // We need to update these arrays to maintain correct references after reordering
+  std::vector<GlobalVariable*> globalArraysToUpdate;
+  for (GlobalVariable &GV : M.globals()) {
+    if (GV.hasInitializer() && (GV.getName() == "llvm.used" || 
+                                GV.getName() == "llvm.compiler.used")) {
+      globalArraysToUpdate.push_back(&GV);
+    }
+  }
+  
+  // Remove all functions from the module temporarily (but keep them alive)
+  // We need to do this carefully to avoid destroying the functions
+  std::vector<Function*> allFunctions;
+  for (Function &F : M) {
+    allFunctions.push_back(&F);
+  }
+  
+  // Clear the function list without destroying the functions
+  auto &FunctionList = M.getFunctionList();
+  for (Function *F : allFunctions) {
+    F->removeFromParent();
+  }
+  
+  // Re-add functions in the correct order: declarations first, then definitions
+  for (Function *F : declarations) {
+    FunctionList.push_back(F);
+  }
+  for (Function *F : definitions) {
+    FunctionList.push_back(F);
+  }
+}
+
 Linker::Linker(Module &M) : Mover(M) {}
 
 bool Linker::linkInModule(

``````````

</details>


https://github.com/llvm/llvm-project/pull/145039