Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 0 additions & 60 deletions clang/lib/Headers/__clang_spirv_builtins.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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;
Comment on lines -96 to -105
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@wenju-he wenju-he Jul 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// OpGenericCastToPtr

Expand All @@ -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 *
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

__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;
Expand All @@ -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;
Expand All @@ -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
Expand Down
Loading