[clang][SPIR-V] Set AS for the SPIR-V logical triple

[bogner]

This was missed in #88455, causing most of the .hlsl to SPIR-V tests to fail (such as clang\test\Driver\hlsl-lang-targets-spirv.hlsl)

[llvmbot]

@llvm/pr-subscribers-clang

Author: Justin Bogner (bogner)

Changes

This was missed in #88455, causing most of the .hlsl to SPIR-V tests to fail (such as clang\test\Driver\hlsl-lang-targets-spirv.hlsl)

---

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

1 Files Affected:

• (modified) clang/lib/Basic/Targets/SPIR.h (+1-1)

diff --git a/clang/lib/Basic/Targets/SPIR.h b/clang/lib/Basic/Targets/SPIR.h
index 9a4a8b501460b6..44265445ff004b 100644
--- a/clang/lib/Basic/Targets/SPIR.h
+++ b/clang/lib/Basic/Targets/SPIR.h
@@ -315,7 +315,7 @@ class LLVM_LIBRARY_VISIBILITY SPIRVTargetInfo : public BaseSPIRVTargetInfo {
     // SPIR-V IDs are represented with a single 32-bit word.
     SizeType = TargetInfo::UnsignedInt;
     resetDataLayout("e-i64:64-v16:16-v24:32-v32:32-v48:64-"
-                    "v96:128-v192:256-v256:256-v512:512-v1024:1024");
+                    "v96:128-v192:256-v256:256-v512:512-v1024:1024-G1");
   }
 
   void getTargetDefines(const LangOptions &Opts,

[Keenuts]

Thanks!

[AlexVlx]

a) Thanks!; b) apologies for the noise; c) this was actually done on purpose, I actively eschewed changing Logical SPIRV because it wasn't actually clear to me if in the long run it'd have the same AS map / would use numerical 1 for globals. If Logical SPIRV is going to go with numerical 1 for globals, LGTM (perhaps it's worth reflecting that in AutoUpgrade.cpp & associated tests, wherein the predicate guards autoupgrades from occuring on Logical SPIRV)!

As a sidenote, as far as I can tell, this flared up because in LLVM's SPIRV TargetMachine there's no segregation between Logical & Physical SPIRV. Would it be worthwhile to add that?

[AlexVlx]

LGTM, please have a peek at the comment if possible.

[bogner]

a) Thanks!; b) apologies for the noise; c) this was actually done on purpose, I actively eschewed changing Logical SPIRV because it wasn't actually clear to me if in the long run it'd have the same AS map / would use numerical 1 for globals. If Logical SPIRV is going to go with numerical 1 for globals, LGTM (perhaps it's worth reflecting that in AutoUpgrade.cpp & associated tests, wherein the predicate guards autoupgrades from occuring on Logical SPIRV)!

As a sidenote, as far as I can tell, this flared up because in LLVM's SPIRV TargetMachine there's no segregation between Logical & Physical SPIRV. Would it be worthwhile to add that?

@Keenuts Do you have thoughts here? I'm not sure there's been much discussion around this.

I'm going to go ahead and push this as is to get the tests passing and we should follow up as necessary.

[coopp]

Looks good

[Keenuts]

Thanks all!
Agree with Bogner, let's unblock the tests first.
As for the address space for globals, this isn't something we have looked into yet, so I'd be in favor of keeping the same behavior as the SPIRN flavor until we have a reason to diverge (as in "thought about this issue" 😊)