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

[Paulius Velesko via llvm-commits]

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

>From 7cc7ac604747b100893cceda7ef3764c3aa71e18 Mon Sep 17 00:00:00 2001
From: Paulius Velesko <pvelesko at pglc.io>
Date: Fri, 20 Jun 2025 14:29:28 +0300
Subject: [PATCH] Fix SPIR-V function ordering violation in linker

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
---
 llvm/lib/Linker/LinkModules.cpp | 60 +++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/llvm/lib/Linker/LinkModules.cpp b/llvm/lib/Linker/LinkModules.cpp
index 485ac106d4ebb..7e4032e366e3a 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,64 @@ 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 (!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
+  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(