https://github.com/jhuber6 closed
https://github.com/llvm/llvm-project/pull/72280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/JonChesterfield approved this pull request.
This is functionally correct and useful as is - if gcc decide to do something
divergent we can change it later, it's basically an internal interface anyway.
Let's have it now and change the names if we come up with better ideas late
JonChesterfield wrote:
The capability is more important than the naming. `__llvm_atomic_scoped_load`
would be fine, with string literals or enum or macro controlling the scope. I
also don't mind if it's a scoped argument or if we end up with
`__llvm_atomic_seqcst_device_load`, embedding all of
https://github.com/jhuber6 updated
https://github.com/llvm/llvm-project/pull/72280
>From ce494cd3f50720b6ba2b8a689f30272c09c06d00 Mon Sep 17 00:00:00 2001
From: Joseph Huber
Date: Mon, 6 Nov 2023 07:08:18 -0600
Subject: [PATCH] [Clang] Introduce scoped variants of GNU atomic functions
Summary:
jhuber6 wrote:
> My primary concerns here are:
>
> * It being likely these builtins will be superseded by something else
> once someone else tries to standardize this. Maybe this isn't a big deal...
> but maybe we want to choose names that are less likely to overlap with stuff
> anyone e
efriedma-quic wrote:
My primary concerns here are:
- It being likely these builtins will be superseded by something else once
someone else tries to standardize this. Maybe this isn't a big deal... but
maybe we want to choose names that are less likely to overlap with stuff anyone
else is doin
https://github.com/jhuber6 updated
https://github.com/llvm/llvm-project/pull/72280
>From b244d36e78cf3e496a41369855e294a6e5765c6d Mon Sep 17 00:00:00 2001
From: Joseph Huber
Date: Mon, 6 Nov 2023 07:08:18 -0600
Subject: [PATCH] [Clang] Introduce scoped variants of GNU atomic functions
Summary:
jhuber6 wrote:
> Missing change to clang/docs/LanguageExtensions.rst describing the new
> builtins.
>
Will do.
> Are there any other projects that we might want to coordinate with here? gcc,
> maybe?
Unknown, I've never collaborated with anyone outside of LLVM. I know they have
handling of G
efriedma-quic wrote:
Missing change to clang/docs/LanguageExtensions.rst describing the new builtins.
Are there any other projects that we might want to coordinate with here? gcc,
maybe?
https://github.com/llvm/llvm-project/pull/72280
___
cfe-commit
jhuber6 wrote:
> We're already assigning names to the different scopes; we're just doing it in
> the `__MEMORY_SCOPE_*` preprocessor macros. Replacing `_MEMORY_SCOPE_SYSTEM`
> in the code with `"system"` doesn't really change the nature of how we assign
> names to scopes.
>
> If these are sup
efriedma-quic wrote:
We're already assigning names to the different scopes; we're just doing it in
the __MEMORY_SCOPE_* preprocessor macros. Replacing `_MEMORY_SCOPE_SYSTEM` in
the code with `"system"` doesn't really change the nature of how we assign
names to scopes.
If these are supposed t
yxsamliu wrote:
> > > The underlying implementation is a string literal in the LLVM syncscope
> > > argument, but the problem is that this isn't standardized at all and
> > > varies between backends potentially
> >
> >
> > We don't have to use the same set of strings as syncscope if that does
jhuber6 wrote:
> > The underlying implementation is a string literal in the LLVM syncscope
> > argument, but the problem is that this isn't standardized at all and varies
> > between backends potentially
>
> We don't have to use the same set of strings as syncscope if that doesn't
> make sens
efriedma-quic wrote:
> The underlying implementation is a string literal in the LLVM syncscope
> argument, but the problem is that this isn't standardized at all and varies
> between backends potentially
We don't have to use the same set of strings as syncscope if that doesn't make
sense.
ht
jhuber6 wrote:
> > I figured we can just treat these as clang extensions for the time being.
> > We already have two variants that are more or less redundant for specific
> > use-cases, (OpenCL and HIP), which should be able to be removed after this.
>
> I'm not sure what you mean here. If you
efriedma-quic wrote:
> I figured we can just treat these as clang extensions for the time being. We
> already have two variants that are more or less redundant for specific
> use-cases, (OpenCL and HIP), which should be able to be removed after this.
I'm not sure what you mean here. If you me
jhuber6 wrote:
> I'm a little wary of adding these without actually going through any sort of
> standardization process; if other vendors don't support the same interface,
> we just have more variations. (See also
> https://clang.llvm.org/get_involved.html#criteria )
>
> How consistent are th
efriedma-quic wrote:
I'm a little wary of adding these without actually going through any sort of
standardization process; if other vendors don't support the same interface, we
just have more variations. (See also
https://clang.llvm.org/get_involved.html#criteria )
How consistent are the var
jhuber6 wrote:
> Overall I think it is the right way to go. Memory scope has been used by
> different offloading languages and the atomic clang builtins are essentially
> the same. Adding a generic clang atomic builtins with memory scope allows
> code sharing among offloading languages.
I agr
yxsamliu wrote:
Overall I think it is the right way to go. Memory scope has been used by
different offloading languages and the atomic clang builtins are essentially
the same. Adding a generic clang atomic builtins with memory scope allows code
sharing among offloading languages.
https://gith
@@ -904,6 +904,32 @@ BUILTIN(__atomic_signal_fence, "vi", "n")
BUILTIN(__atomic_always_lock_free, "bzvCD*", "nE")
BUILTIN(__atomic_is_lock_free, "bzvCD*", "nE")
+// GNU atomic builtins with atomic scopes.
+ATOMIC_BUILTIN(__scoped_atomic_load, "v.", "t")
jhuber
@@ -54,6 +59,16 @@ enum class SyncScope {
inline llvm::StringRef getAsString(SyncScope S) {
jhuber6 wrote:
I think it's because this is for AST printing purposes, while the backend
strings vary per target.
https://github.com/llvm/llvm-project/pull/72280
@@ -205,6 +220,56 @@ class AtomicScopeHIPModel : public AtomicScopeModel {
}
};
+/// Defines the generic atomic scope model.
+class AtomicScopeGenericModel : public AtomicScopeModel {
+public:
+ /// The enum values match predefined built-in macros __ATOMIC_SCOPE_*.
+ enum
@@ -798,6 +798,13 @@ static void InitializePredefinedMacros(const TargetInfo
&TI,
Builder.defineMacro("__ATOMIC_ACQ_REL", "4");
Builder.defineMacro("__ATOMIC_SEQ_CST", "5");
+ // Define macros for the clang atomic scopes.
+ Builder.defineMacro("__MEMORY_SCOPE_SYSTEM", "
jhuber6 wrote:
> Is there any actual difference now between these and the HIP/OpenCL flavors
> other than dropping the language from the name?
Yes, these directly copy the GNU functions and names. The OpenCL / HIP ones use
a different format.
https://github.com/llvm/llvm-project/pull/72280
__
@@ -904,6 +904,32 @@ BUILTIN(__atomic_signal_fence, "vi", "n")
BUILTIN(__atomic_always_lock_free, "bzvCD*", "nE")
BUILTIN(__atomic_is_lock_free, "bzvCD*", "nE")
+// GNU atomic builtins with atomic scopes.
+ATOMIC_BUILTIN(__scoped_atomic_load, "v.", "t")
arsenm
@@ -54,6 +59,16 @@ enum class SyncScope {
inline llvm::StringRef getAsString(SyncScope S) {
arsenm wrote:
I guess this is a pre-existing problem, but why don't these just match the
backend string names?
https://github.com/llvm/llvm-project/pull/72280
___
@@ -205,6 +220,56 @@ class AtomicScopeHIPModel : public AtomicScopeModel {
}
};
+/// Defines the generic atomic scope model.
+class AtomicScopeGenericModel : public AtomicScopeModel {
+public:
+ /// The enum values match predefined built-in macros __ATOMIC_SCOPE_*.
+ enum
https://github.com/arsenm edited https://github.com/llvm/llvm-project/pull/72280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -798,6 +798,13 @@ static void InitializePredefinedMacros(const TargetInfo
&TI,
Builder.defineMacro("__ATOMIC_ACQ_REL", "4");
Builder.defineMacro("__ATOMIC_SEQ_CST", "5");
+ // Define macros for the clang atomic scopes.
+ Builder.defineMacro("__MEMORY_SCOPE_SYSTEM", "
https://github.com/arsenm commented:
Is there any actual difference now between these and the HIP/OpenCL flavors
other than dropping the language from the name?
https://github.com/llvm/llvm-project/pull/72280
___
cfe-commits mailing list
cfe-commits@
https://github.com/arsenm edited https://github.com/llvm/llvm-project/pull/72280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Artem-B wrote:
> Nvidia backend doesn't handle scoped atomics at all yet
Yeah, it's on my ever growing todo. :-(
https://github.com/llvm/llvm-project/pull/72280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/m
jhuber6 wrote:
> Just a FYI, that recent NVIDIA GPUs have introduced a concept of [thread
> block
> cluster](https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html#thread-block-clusters).
> We may need another level of granularity between the block and device.
Should be easy enough,
Artem-B wrote:
Just a FYI, that recent NVIDIA GPUs have introduced a concept of [thread block
cluster](https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html#thread-block-clusters).
We may need another level of granularity between the block and device.
https://github.com/llvm/llvm-
JonChesterfield wrote:
Looks solid to me. The patch to clang is long but straightforward and the tests
look reassuringly exhaustive. Probably good that you ignored my name suggestion
of integers 0 through N.
This patch is partly motivated by us wanting device scope atomics in libc. It
removes
https://github.com/JonChesterfield edited
https://github.com/llvm/llvm-project/pull/72280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
llvmbot wrote:
@llvm/pr-subscribers-backend-amdgpu
Author: Joseph Huber (jhuber6)
Changes
Summary:
The standard GNU atomic operations are a very common way to target
hardware atomics on the device. With more hetergenous devices being
introduced, the concept of memory scopes has been in the
llvmbot wrote:
@llvm/pr-subscribers-clang-codegen
Author: Joseph Huber (jhuber6)
Changes
Summary:
The standard GNU atomic operations are a very common way to target
hardware atomics on the device. With more hetergenous devices being
introduced, the concept of memory scopes has been in the
https://github.com/jhuber6 created
https://github.com/llvm/llvm-project/pull/72280
Summary:
The standard GNU atomic operations are a very common way to target
hardware atomics on the device. With more hetergenous devices being
introduced, the concept of memory scopes has been in the LLVM languag
40 matches
Mail list logo