Fix SPIR-V function ordering violation in linker #145039
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-lto Author: Paulius Velesko (pvelesko) ChangesSPIR-V specification requires all function declarations to appear before Problem:
Solution:
The fix ensures SPIR-V modules comply with the specification while Fixes linking errors for HIP/OpenCL programs targeting SPIR-V. Full diff: https://github.com/llvm/llvm-project/pull/145039.diff 1 Files Affected:
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(
|
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
|
I don’t think fixing the SPIR-V function ordering in the LLVM IR linker is the right place and it’s SPIR-V emitters’ responsibility (e.g. SPIR-V backend and LLVM-SPIRV-Translator) to emit valid SPIR-V code - IOW, the fix should probably done in the emitters. Also, there is a danger that the reordering done in the linker may be undone by follow up transformation passes. |
|
@linehill emitter is properly emitting everything. The linker fails to properly stitch them together at link time. |
that seems like a separate issue. |
The LLVM IR linker fails? In what way? Because of the resulting function definition and declaration order in LLVM IR module? |
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:
Solution:
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.