[clang][SPIRV] Remove volatile variants of GenericCastToPtr* built-ins

[wenju-he]

They are generally implemented with pointer comparison, e.g checking
pointer value is within a range of a specific address space. Volatile
is not needed because memory pointed to by the pointer isn't accessed.
With this PR, backend target doesn't need to implement volatile variant.

[llvmbot]

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-clang

Author: Wenju He (wenju-he)

Changes

They are generally implemented with pointer comparison. Volatile is not needed because memory pointed to by the pointer isn't accessed.

---

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

1 Files Affected:

• (modified) clang/lib/Headers/__clang_spirv_builtins.h (-60)

diff --git a/clang/lib/Headers/__clang_spirv_builtins.h b/clang/lib/Headers/__clang_spirv_builtins.h
index e344ed52571a7..c210609ad5973 100644
--- a/clang/lib/Headers/__clang_spirv_builtins.h
+++ b/clang/lib/Headers/__clang_spirv_builtins.h
@@ -56,16 +56,6 @@ __spirv_GenericCastToPtrExplicit_ToGlobal(__generic const void *,
                                           int) __SPIRV_NOEXCEPT;
 extern __SPIRV_overloadable
 __SPIRV_BUILTIN_ALIAS(__builtin_spirv_generic_cast_to_ptr_explicit)
-__global volatile void *
-__spirv_GenericCastToPtrExplicit_ToGlobal(__generic volatile void *,
-                                          int) __SPIRV_NOEXCEPT;
-extern __SPIRV_overloadable
-__SPIRV_BUILTIN_ALIAS(__builtin_spirv_generic_cast_to_ptr_explicit)
-__global const volatile void *
-__spirv_GenericCastToPtrExplicit_ToGlobal(__generic const volatile void *,
-                                          int) __SPIRV_NOEXCEPT;
-extern __SPIRV_overloadable
-__SPIRV_BUILTIN_ALIAS(__builtin_spirv_generic_cast_to_ptr_explicit)
 __local void *__spirv_GenericCastToPtrExplicit_ToLocal(__generic void *,
                                                        int) __SPIRV_NOEXCEPT;
 extern __SPIRV_overloadable
@@ -75,16 +65,6 @@ __spirv_GenericCastToPtrExplicit_ToLocal(__generic const void *,
                                          int) __SPIRV_NOEXCEPT;
 extern __SPIRV_overloadable
 __SPIRV_BUILTIN_ALIAS(__builtin_spirv_generic_cast_to_ptr_explicit)
-__local volatile void *
-__spirv_GenericCastToPtrExplicit_ToLocal(__generic volatile void *,
-                                         int) __SPIRV_NOEXCEPT;
-extern __SPIRV_overloadable
-__SPIRV_BUILTIN_ALIAS(__builtin_spirv_generic_cast_to_ptr_explicit)
-__local const volatile void *
-__spirv_GenericCastToPtrExplicit_ToLocal(__generic const volatile void *,
-                                         int) __SPIRV_NOEXCEPT;
-extern __SPIRV_overloadable
-__SPIRV_BUILTIN_ALIAS(__builtin_spirv_generic_cast_to_ptr_explicit)
 __private void *
 __spirv_GenericCastToPtrExplicit_ToPrivate(__generic void *,
                                            int) __SPIRV_NOEXCEPT;
@@ -93,16 +73,6 @@ __SPIRV_BUILTIN_ALIAS(__builtin_spirv_generic_cast_to_ptr_explicit)
 __private const void *
 __spirv_GenericCastToPtrExplicit_ToPrivate(__generic const void *,
                                            int) __SPIRV_NOEXCEPT;
-extern __SPIRV_overloadable
-__SPIRV_BUILTIN_ALIAS(__builtin_spirv_generic_cast_to_ptr_explicit)
-__private volatile void *
-__spirv_GenericCastToPtrExplicit_ToPrivate(__generic volatile void *,
-                                           int) __SPIRV_NOEXCEPT;
-extern __SPIRV_overloadable
-__SPIRV_BUILTIN_ALIAS(__builtin_spirv_generic_cast_to_ptr_explicit)
-__private const volatile void *
-__spirv_GenericCastToPtrExplicit_ToPrivate(__generic const volatile void *,
-                                           int) __SPIRV_NOEXCEPT;
 
 // OpGenericCastToPtr
 
@@ -115,16 +85,6 @@ __spirv_GenericCastToPtr_ToGlobal(__generic const void *p,
                                   int) __SPIRV_NOEXCEPT {
   return (__global const void *)p;
 }
-static __SPIRV_overloadable __SPIRV_inline __global volatile void *
-__spirv_GenericCastToPtr_ToGlobal(__generic volatile void *p,
-                                  int) __SPIRV_NOEXCEPT {
-  return (__global volatile void *)p;
-}
-static __SPIRV_overloadable __SPIRV_inline __global const volatile void *
-__spirv_GenericCastToPtr_ToGlobal(__generic const volatile void *p,
-                                  int) __SPIRV_NOEXCEPT {
-  return (__global const volatile void *)p;
-}
 static __SPIRV_overloadable __SPIRV_inline __local void *
 __spirv_GenericCastToPtr_ToLocal(__generic void *p, int) __SPIRV_NOEXCEPT {
   return (__local void *)p;
@@ -134,16 +94,6 @@ __spirv_GenericCastToPtr_ToLocal(__generic const void *p,
                                  int) __SPIRV_NOEXCEPT {
   return (__local const void *)p;
 }
-static __SPIRV_overloadable __SPIRV_inline __local volatile void *
-__spirv_GenericCastToPtr_ToLocal(__generic volatile void *p,
-                                 int) __SPIRV_NOEXCEPT {
-  return (__local volatile void *)p;
-}
-static __SPIRV_overloadable __SPIRV_inline __local const volatile void *
-__spirv_GenericCastToPtr_ToLocal(__generic const volatile void *p,
-                                 int) __SPIRV_NOEXCEPT {
-  return (__local const volatile void *)p;
-}
 static __SPIRV_overloadable __SPIRV_inline __private void *
 __spirv_GenericCastToPtr_ToPrivate(__generic void *p, int) __SPIRV_NOEXCEPT {
   return (__private void *)p;
@@ -153,16 +103,6 @@ __spirv_GenericCastToPtr_ToPrivate(__generic const void *p,
                                    int) __SPIRV_NOEXCEPT {
   return (__private const void *)p;
 }
-static __SPIRV_overloadable __SPIRV_inline __private volatile void *
-__spirv_GenericCastToPtr_ToPrivate(__generic volatile void *p,
-                                   int) __SPIRV_NOEXCEPT {
-  return (__private volatile void *)p;
-}
-static __SPIRV_overloadable __SPIRV_inline __private const volatile void *
-__spirv_GenericCastToPtr_ToPrivate(__generic const volatile void *p,
-                                   int) __SPIRV_NOEXCEPT {
-  return (__private const volatile void *)p;
-}
 
 #undef __SPIRV_overloadable
 #undef __SPIRV_convergent

[Naghasan]

While I agree with your point, that imposes a cast for the user. How about providing the implementation for it then ?

[wenju-he]

While I agree with your point, that imposes a cast for the user.

the cast at user site is only necessary if the input pointer argument is already volatile, right? In such case, any built-in that uses the pointer arg will have the same issue, i.e. casting is needed. So it is a general issue.

IMO whether volatile is present should reflect how the built-in uses the pointer argument, similar as when we adds an specific attribute for a function argument.

[Naghasan]

In such case, any built-in that uses the pointer arg will have the same issue,

Not sure what you mean. Did you mean the returned value ?

IMO whether volatile is present should reflect how the built-in uses the pointer argument

Yes and no, there are situation where we should ban certain overload because it doesn't make sense (e.g. no const if we write the memory) but we also want a certain level of flexibility. Taking your point to the letter, the only overload we need here is the one taking / returning const, but that would force users to cast away the const (which usually raises a few eyebrows...).

I don't have strong opinion, a cast on the return type will usually be required, but I find it better to not have to deal with cv-quals.

[wenju-he]

In such case, any built-in that uses the pointer arg will have the same issue,

Not sure what you mean.

I mean the case that the input pointer to builtin is already volatile, e.g. test2 shown below. Suppose the pointer parameter of __spirv_ocl_vloadn_Rint2 isn't qualified with volatile.
Before this PR,

void test1(int *p) {
  __spirv_GenericCastToPtrExplicit_ToPrivate(p); // const-cast not needed
  __spirv_ocl_vloadn_Rint2(1, p); // const-cast not needed
}
void test2(volatile int *p) {
  __spirv_GenericCastToPtrExplicit_ToPrivate(p); // const-cast not needed
  __spirv_ocl_vloadn_Rint2(1, p); // const-cast needed
}

after this PR,

void test1(int *p) {
  __spirv_GenericCastToPtrExplicit_ToPrivate(p); // const-cast not needed
  __spirv_ocl_vloadn_Rint2(1, p); // const-cast not needed
}
void test2(volatile int *p) {
  __spirv_GenericCastToPtrExplicit_ToPrivate(p); // const-cast needed   <-- changed
  __spirv_ocl_vloadn_Rint2(1, p); // const-cast needed
}

I mean after this PR __spirv_GenericCastToPtrExplicit_ToPrivate will behave the same as other built-in that uses pointer argument regarding whether a const-cast is needed. So the casting is a general issue.
If we keep volatile variant of __spirv_GenericCastToPtrExplicit_ToPrivate, does it mean we'll add volatile overload for all built-ins that has pointer parameter?

Did you mean the returned value ?
a cast on the return type will usually be required

Cast on return value is only need for const, but not needed for volatile after this PR, right? I mean a non-volatile return value can be implicitly converted to a volatile value.

I don't have strong opinion

Me neither. It is just we probably need to find a correct way to handle the builtin declarations.

[Naghasan]

Why removing these ? No binding it required as it boils down to an addrspacecast insn.

[wenju-he]

sorry I didn't realize these are static functions. They are not bindings, right?

[Naghasan]

yeah, no bindings required as they translate to an LLVM insn.