[SPIR-V] Fix an undefined behaviour in the SPIRV emit-intrinsics pass

[VyacheslavLevytskyy]

Adding SPIRV to LLVM_ALL_TARGETS (#119653) revealed a series of minor compilation problems and sanitizer complaints. This PR is to address the problem of the undefined behaviour in the SPIRV emit-intrinsics pass.

I can see no more SPIR-V Builder-related fails in sanitizers, checked locally as:

mkdir sanitizer
cd sanitizer/
git clone https://github.com/llvm/llvm-zorg.git
BUILDBOT_MONO_REPO_PATH=/path-to-the-fixed-locally-sources/llvm-project llvm-zorg/zorg/buildbot/builders/sanitizers/buildbot_fast.sh

[llvmbot]

@llvm/pr-subscribers-backend-spir-v

Author: Vyacheslav Levytskyy (VyacheslavLevytskyy)

Changes

Adding SPIRV to LLVM_ALL_TARGETS (#119653) revealed a series of minor compilation problems and sanitizer complaints. This PR is to address the problem of the undefined behaviour in the SPIRV emit-intrinsics pass.

I can see no more SPIR-V Builder-related fails in sanitizers, checked locally as:

mkdir sanitizer
cd sanitizer/
git clone https://github.com/llvm/llvm-zorg.git
BUILDBOT_MONO_REPO_PATH=/path-to-the-fixed-locally-sources/llvm-project llvm-zorg/zorg/buildbot/builders/sanitizers/buildbot_fast.sh

---

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

1 Files Affected:

• (modified) llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp (+4-2)

diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 1c1acd29ee0e6a..c77b9af4652b95 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -2475,8 +2475,10 @@ void SPIRVEmitIntrinsics::parseFunDeclarations(Module &M) {
     if (DemangledName.empty())
       continue;
     // allow only OpGroupAsyncCopy use case at the moment
-    auto [Grp, Opcode, ExtNo] =
-        SPIRV::mapBuiltinToOpcode(DemangledName, InstrSet);
+    auto [Grp, Opcode, ExtNo] = SPIRV::mapBuiltinToOpcode(
+        DemangledName, TM->getSubtarget<SPIRVSubtarget>(F).isOpenCLEnv()
+                           ? SPIRV::InstructionSet::OpenCL_std
+                           : SPIRV::InstructionSet::GLSL_std_450);
     if (Opcode != SPIRV::OpGroupAsyncCopy)
       continue;
     // find pointer arguments

[Keenuts]

Should this variable be removed from the class?

[VyacheslavLevytskyy]

Not necessarily, it is in use in one more piece of the code in deduceOperandElementTypeCalledFunction(). However, it may make sense indeed, because it's the only use of it, so caching improves nothing.

[VyacheslavLevytskyy]

Updated, removing the class member InstrSet

[michalpaszkowski]

I started working on a similar change yesterday evening, but refactored getting the instruction set into a separate function and removed some repetition in another place: #123625

[VyacheslavLevytskyy]

I started working on a similar change yesterday evening, but refactored getting the instruction set into a separate function and removed some repetition in another place: #123625

Yes, it's even better, thank you

[VyacheslavLevytskyy]

close as a duplicate of #123625