search

KhronosGroup / SPIRV-LLVM-Translator

A tool and a library for bi-directional translation between SPIR-V and LLVM IR
Other
479 stars 213 forks source link

Drop llvm.compiler.used GV? #2568

Open MrSidims opened 4 months ago

MrSidims commented 4 months ago

Before https://github.com/llvm/llvm-project/commit/1120d8e6f799 compiler FE would place llvm.compiler.used GV in private address space and translator would error out facing it due to recently added diagnostics. Before this diagnostic we were translating it to SPIR-V module to OpVariable with llvm.compiler.used name and function storage class and in SPIRVReader we would drop this GV completely, see https://github.com/KhronosGroup/SPIRV-LLVM-Translator/blob/main/lib/SPIRV/SPIRVReader.cpp#L3442 .

Right now with the mentioned community patch translator no longer drop the GV and it survives the translation. But reverse translation would not be 100% correct, we also need to change section of this GV to llvm.metadata. We could add the section, but considering definition from the LangRef:

On targets that support it, this allows an intelligent linker to optimize references to the symbol without being impeded as it would be by @llvm.used.

so strictly speaking, our target here is SPIR-V and SPIR-V doesn't have support for such magic GV, so going to non-necessarily-LLVM-based backend such GV might remain to be unresolved (an we don't have appending linkage type in SPIR-V to help with properly resolving the GV). And IMHO it's better to not generate OpVariable when facing llvm.compiler.used at all also considering that relying on its information passed for optimized from FE to device's compiler is a bad idea. WDYT?

svenvh commented 4 months ago

It seems there is currently no strong use case anyway for these "meta" variables, so dropping them sounds reasonable.