-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[SPIRV] Do not emit @llvm.compiler.used #162678
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-backend-spir-v Author: Juan Manuel Martinez Caamaño (jmmartinez) Changes
However, the symbols in the variable can be optimized after compilation This was already done for Full diff: https://github.com/llvm/llvm-project/pull/162678.diff 2 Files Affected:
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index e16c8f0fc302e..12b2f558bf383 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -17,6 +17,7 @@
#include "SPIRVTargetMachine.h"
#include "SPIRVUtils.h"
#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/StringSet.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/InstIterator.h"
#include "llvm/IR/InstVisitor.h"
@@ -2028,9 +2029,13 @@ Instruction *SPIRVEmitIntrinsics::visitUnreachableInst(UnreachableInst &I) {
void SPIRVEmitIntrinsics::processGlobalValue(GlobalVariable &GV,
IRBuilder<> &B) {
- // Skip special artifical variable llvm.global.annotations.
- if (GV.getName() == "llvm.global.annotations")
+ // Skip special artifical variables
+ static const StringSet<> ArtificialGlobals{"llvm.global.annotations",
+ "llvm.compiler.used"};
+
+ if (ArtificialGlobals.contains(GV.getName()))
return;
+
Constant *Init = nullptr;
if (hasInitializer(&GV)) {
// Deduce element type and store results in Global Registry.
diff --git a/llvm/test/CodeGen/SPIRV/llvm-compiler-used.ll b/llvm/test/CodeGen/SPIRV/llvm-compiler-used.ll
new file mode 100644
index 0000000000000..3a46c8d150b62
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/llvm-compiler-used.ll
@@ -0,0 +1,18 @@
+; RUN: llc -verify-machineinstrs -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -mtriple=spirv-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; Verify that llvm.compiler.used is not lowered
+; Currently we do lower it.
+; CHECK: OpName %[[UNUSED:[0-9]+]] "unused"
+; CHECK-NOT: OpName %[[LLVM_COMPILER_USED_NAME:[0-9]+]] "llvm.compiler.used"
+
+; Check that the type of llvm.compiler.used is not emited too
+; CHECK-NOT: OpTypeArray
+
+@unused = private addrspace(3) global i32 0
+@llvm.compiler.used = appending addrspace(2) global [1 x ptr addrspace (4)] [ptr addrspace(4) addrspacecast (ptr addrspace(3) @unused to ptr addrspace(4))]
+
+define spir_kernel void @foo() {
+entry:
+ ret void
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
✅ With the latest revision this PR passed the C/C++ code formatter. |
static const StringSet<> ArtificialGlobals{"llvm.global.annotations", | ||
"llvm.used", "llvm.compiler.used"}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these just all the cases with appending linkage? Probably should not use this set though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think that all these special variables will eventually need some special handling. I'll stick on the easy cases while I gain more experience with the whole spirv environment.
llvm/test/CodeGen/SPIRV/llvm-used.ll
Outdated
; RUN: llc -verify-machineinstrs -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s | ||
; RUN: %if spirv-tools %{ llc -mtriple=spirv-unknown-unknown %s -o - -filetype=obj | spirv-val %} | ||
|
||
; Verify that llvm.used is not lowered |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should not just be droppe. I'd expect there to be some spirv equivalent mechanism for preserving unused symbols which should be emitted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to make sure that no tools (spirv-link and spirv-opt) remove the global variables, then the only way to do that is to give the variable the export linkage decoration. However, the append
semantics will be lost, so I'm not sure if the spir-v linker will deal with it correctly.
I think it is fair to say that llvm.used
cannot be supported, and it is not accepted by the SPIR-V backend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we can meaningfully do anything different for llvm.compiler.used
and llvm.used
.
What is the use case? Which tools are you expecting to be able to use after compiling?
llvm/test/CodeGen/SPIRV/llvm-used.ll
Outdated
; RUN: llc -verify-machineinstrs -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s | ||
; RUN: %if spirv-tools %{ llc -mtriple=spirv-unknown-unknown %s -o - -filetype=obj | spirv-val %} | ||
|
||
; Verify that llvm.used is not lowered |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to make sure that no tools (spirv-link and spirv-opt) remove the global variables, then the only way to do that is to give the variable the export linkage decoration. However, the append
semantics will be lost, so I'm not sure if the spir-v linker will deal with it correctly.
I think it is fair to say that llvm.used
cannot be supported, and it is not accepted by the SPIR-V backend.
This variable holds a series of global values and prevents the compiler from optimizing it out. However, the symbols in the variable can be optimized after compilation as usual by the linker (notice that @llvm.used instead doesn't allow for the linker to optimize out the globals referenced in it).
fe16a73
to
e0bfd9b
Compare
I'll restrict this PR to At the moment, I'm only interested in |
@llvm.compiler.used
holds a series of global values and prevents the compilerfrom optimizing it out.
However, the symbols in the variable can be optimized after compilation
as usual by a linker (notice that
@llvm.used
instead doesn't allow forthe linker to optimize-out the global variables referenced in it).
This was already done for
@llvm.global.annotations
.