[SPIR-V] Permit implicit conversion to generic AS

[jhuber6]

Summary:
We rely on this in most places we work with address spaces. This allows
target address spaces to implicity convert to generic ones.

I actually have no clue if this is valid or correct with SPIR-V, hoping
someone with more target / backend knowledge can chime in.

[llvmbot]

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

@llvm/pr-subscribers-clang

Author: Joseph Huber (jhuber6)

Changes

Summary:
We rely on this in most places we work with address spaces. This allows
target address spaces to implicity convert to generic ones.

I actually have no clue if this is valid or correct with SPIR-V, hoping
someone with more target / backend knowledge can chime in.

---

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

2 Files Affected:

• (modified) clang/lib/Basic/Targets/SPIR.h (+11)
• (added) clang/test/Sema/spirv-address-space.c (+21)

diff --git a/clang/lib/Basic/Targets/SPIR.h b/clang/lib/Basic/Targets/SPIR.h
index d5374602caeaa..d72531eab9177 100644
--- a/clang/lib/Basic/Targets/SPIR.h
+++ b/clang/lib/Basic/Targets/SPIR.h
@@ -14,6 +14,7 @@
 #define LLVM_CLANG_LIB_BASIC_TARGETS_SPIR_H
 
 #include "Targets.h"
+#include "clang/Basic/AddressSpaces.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
 #include "llvm/Support/Compiler.h"
@@ -317,6 +318,16 @@ class LLVM_LIBRARY_VISIBILITY BaseSPIRVTargetInfo : public BaseSPIRTargetInfo {
     return Feature == "spirv";
   }
 
+  virtual bool isAddressSpaceSupersetOf(LangAS A, LangAS B) const override {
+    // The geneirc space AS(4) is a superset of all the other address
+    // spaces used by the backend target.
+    return A == B || ((A == LangAS::Default ||
+                       (isTargetAddressSpace(A) &&
+                        toTargetAddressSpace(A) == /*Generic=*/4)) &&
+                      isTargetAddressSpace(B) &&
+                      toTargetAddressSpace(B) <= /*Generic=*/4);
+  }
+
   void getTargetDefines(const LangOptions &Opts,
                         MacroBuilder &Builder) const override;
 };
diff --git a/clang/test/Sema/spirv-address-space.c b/clang/test/Sema/spirv-address-space.c
new file mode 100644
index 0000000000000..7ec41fd79bd96
--- /dev/null
+++ b/clang/test/Sema/spirv-address-space.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -triple spirv64 -fsyntax-only -verify
+
+#define _AS0 __attribute__((address_space(0)))
+#define _AS1 __attribute__((address_space(1)))
+#define _AS2 __attribute__((address_space(2)))
+#define _AS3 __attribute__((address_space(3)))
+#define _AS4 __attribute__((address_space(4)))
+#define _AS5 __attribute__((address_space(5)))
+#define _AS999 __attribute__((address_space(999)))
+
+void *p1(void _AS1 *p) { return p; } 
+void *p2(void _AS2 *p) { return p; } 
+void *p3(void _AS3 *p) { return p; }
+void *p4(void _AS4 *p) { return p; }
+void *p5(void _AS5 *p) { return p; } // expected-error {{returning '_AS5 void *' from a function with result type 'void *' changes address space of pointer}}
+void *pi(void _AS999 *p) { return p; } // expected-error {{returning '_AS999 void *' from a function with result type 'void *' changes address space of pointer}}
+void *pc(void __attribute__((opencl_local)) *p) { return p; } // expected-error {{returning '__local void *' from a function with result type 'void *' changes address space of pointer}}
+void _AS1 *r0(void _AS1 *p) { return p; }
+void _AS1 *r1(void *p) { return p; } // expected-error {{returning 'void *' from a function with result type '_AS1 void *' changes address space of pointer}}
+void _AS1 *r2(void *p) { return (void _AS1 *)p; }
+

[github-actions]

🐧 Linux x64 Test Results

• 112578 tests passed
• 4600 tests skipped

✅ The build succeeded and all tests passed.

[arsenm]

I guess no progress has been made on the effort to fix using 4 for generic?

[arsenm]

Use enum?

[jhuber6]

SPIR-V doesn't have any enums for this, their backend just hard-codes is everywhere AFAICT so fixing that is a little out of scope.

[sarnex]

The only exception I know is target AS 9 can't be casted to 4 or any other AS, it's the program/function pointer AS, but it looks like this patch won't allow that so LGTM

[jhuber6]

The only exception I know is target AS 9 can't be casted to 4 or any other AS, it's the program/function pointer AS, but it looks like this patch won't allow that so LGTM

Do you if the default AS outside of OpenCL mode (AKA SPIR-V) behaves like Generic? I'm assuming SPIR-V behaves that way.

[sarnex]

My understanding is that the Generic/Default semantics are the same for any offloading language.

[AlexVlx]

The only exception I know is target AS 9 can't be casted to 4 or any other AS, it's the program/function pointer AS, but it looks like this patch won't allow that so LGTM

This is (sadly) not correct; for example Constant (2) is also not castable, per SPIR-V spec. Relaxing this might be beneficial, and it might be worth looking at an extension, but technically this change will potentially yield invalid SPIR-V. Please consult the SPIR-V spec for OpPtrCastToGeneric and OpGenericCastToPtr. Please consider either reverting, or at least handling Constant, to avoid that. Do note that there are a bunch of other StorageClasses (ASes in LLVM parlance) that SPIR-V can express, which also are not castable to Generic, and which might be used by HLSL since they are GFX specific - perhaps it is safer to not override this for the logical SPIRV target info, rather than trying to figure that out.