* [RFC PATCH v1 0/2] Deduplicating RISCV cmpxchg.h macros
@ 2023-04-06 8:20 Leonardo Bras
2023-04-06 8:20 ` [RFC PATCH v1 1/2] riscv/cmpxchg: Deduplicate cmpxchg() asm and macros Leonardo Bras
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Leonardo Bras @ 2023-04-06 8:20 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Leonardo Bras, Guo Ren,
Andrea Parri, Conor Dooley
Cc: linux-riscv, linux-kernel
While studying riscv's cmpxchg.h file, I got really interested in
understanding how RISCV asm implemented the different versions of
{cmp,}xchg.
When I understood the pattern, it made sense for me to remove the
duplications and create macros to make it easier to understand what exactly
changes between the versions: Instruction sufixes & barriers.
Thanks!
Leo
Changes since RFCv3:
- Squashed the 6 original patches in 2: one for cmpxchg and one for xchg
https://lore.kernel.org/all/20230404163741.2762165-1-leobras@redhat.com/
Changes since RFCv2:
- Fixed macros that depend on having a local variable with a magic name
- Previous cast to (long) is now only applied on 4-bytes cmpxchg
https://lore.kernel.org/all/20230321074249.2221674-1-leobras@redhat.com/
Changes since RFCv1:
- Fixed patch 4/6 suffix from 'w.aqrl' to '.w.aqrl', to avoid build error
https://lore.kernel.org/all/20230318080059.1109286-1-leobras@redhat.com/
Leonardo Bras (2):
riscv/cmpxchg: Deduplicate cmpxchg() asm and macros
riscv/cmpxchg: Deduplicate xchg() asm functions
arch/riscv/include/asm/cmpxchg.h | 319 +++++++------------------------
1 file changed, 67 insertions(+), 252 deletions(-)
--
2.40.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH v1 1/2] riscv/cmpxchg: Deduplicate cmpxchg() asm and macros
2023-04-06 8:20 [RFC PATCH v1 0/2] Deduplicating RISCV cmpxchg.h macros Leonardo Bras
@ 2023-04-06 8:20 ` Leonardo Bras
2023-04-07 8:29 ` Guo Ren
2023-04-06 8:20 ` [RFC PATCH v1 2/2] riscv/cmpxchg: Deduplicate xchg() asm functions Leonardo Bras
2023-04-06 17:19 ` [RFC PATCH v1 0/2] Deduplicating RISCV cmpxchg.h macros Andrea Parri
2 siblings, 1 reply; 10+ messages in thread
From: Leonardo Bras @ 2023-04-06 8:20 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Leonardo Bras, Guo Ren,
Andrea Parri, Conor Dooley
Cc: linux-riscv, linux-kernel
In this header every cmpxchg define (_relaxed, _acquire, _release,
vanilla) contain it's own asm file, both for 4-byte variables an 8-byte
variables, on a total of 8 versions of mostly the same asm.
This is usually bad, as it means any change may be done in up to 8
different places.
Unify those versions by creating a new define with enough parameters to
generate any version of the previous 8.
Then unify the result under a more general define, and simplify
arch_cmpxchg* generation
(This did not cause any change in generated asm)
Signed-off-by: Leonardo Bras <leobras@redhat.com>
---
arch/riscv/include/asm/cmpxchg.h | 184 ++++++-------------------------
1 file changed, 36 insertions(+), 148 deletions(-)
diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
index 12debce235e52..f88fae357071c 100644
--- a/arch/riscv/include/asm/cmpxchg.h
+++ b/arch/riscv/include/asm/cmpxchg.h
@@ -163,51 +163,23 @@
* store NEW in MEM. Return the initial value in MEM. Success is
* indicated by comparing RETURN with OLD.
*/
-#define __cmpxchg_relaxed(ptr, old, new, size) \
-({ \
- __typeof__(ptr) __ptr = (ptr); \
- __typeof__(*(ptr)) __old = (old); \
- __typeof__(*(ptr)) __new = (new); \
- __typeof__(*(ptr)) __ret; \
- register unsigned int __rc; \
- switch (size) { \
- case 4: \
- __asm__ __volatile__ ( \
- "0: lr.w %0, %2\n" \
- " bne %0, %z3, 1f\n" \
- " sc.w %1, %z4, %2\n" \
- " bnez %1, 0b\n" \
- "1:\n" \
- : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
- : "rJ" ((long)__old), "rJ" (__new) \
- : "memory"); \
- break; \
- case 8: \
- __asm__ __volatile__ ( \
- "0: lr.d %0, %2\n" \
- " bne %0, %z3, 1f\n" \
- " sc.d %1, %z4, %2\n" \
- " bnez %1, 0b\n" \
- "1:\n" \
- : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
- : "rJ" (__old), "rJ" (__new) \
- : "memory"); \
- break; \
- default: \
- BUILD_BUG(); \
- } \
- __ret; \
-})
-#define arch_cmpxchg_relaxed(ptr, o, n) \
+#define ____cmpxchg(lr_sfx, sc_sfx, prepend, append, r, rc, p, co, o, n)\
({ \
- __typeof__(*(ptr)) _o_ = (o); \
- __typeof__(*(ptr)) _n_ = (n); \
- (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr), \
- _o_, _n_, sizeof(*(ptr))); \
+ __asm__ __volatile__ ( \
+ prepend \
+ "0: lr" lr_sfx " %0, %2\n" \
+ " bne %0, %z3, 1f\n" \
+ " sc" sc_sfx " %1, %z4, %2\n" \
+ " bnez %1, 0b\n" \
+ append \
+ "1:\n" \
+ : "=&r" (r), "=&r" (rc), "+A" (*(p)) \
+ : "rJ" (co o), "rJ" (n) \
+ : "memory"); \
})
-#define __cmpxchg_acquire(ptr, old, new, size) \
+#define ___cmpxchg(ptr, old, new, size, sc_sfx, prepend, append) \
({ \
__typeof__(ptr) __ptr = (ptr); \
__typeof__(*(ptr)) __old = (old); \
@@ -216,28 +188,12 @@
register unsigned int __rc; \
switch (size) { \
case 4: \
- __asm__ __volatile__ ( \
- "0: lr.w %0, %2\n" \
- " bne %0, %z3, 1f\n" \
- " sc.w %1, %z4, %2\n" \
- " bnez %1, 0b\n" \
- RISCV_ACQUIRE_BARRIER \
- "1:\n" \
- : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
- : "rJ" ((long)__old), "rJ" (__new) \
- : "memory"); \
+ ____cmpxchg(".w", ".w" sc_sfx, prepend, append, \
+ __ret, __rc, __ptr, (long), __old, __new); \
break; \
case 8: \
- __asm__ __volatile__ ( \
- "0: lr.d %0, %2\n" \
- " bne %0, %z3, 1f\n" \
- " sc.d %1, %z4, %2\n" \
- " bnez %1, 0b\n" \
- RISCV_ACQUIRE_BARRIER \
- "1:\n" \
- : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
- : "rJ" (__old), "rJ" (__new) \
- : "memory"); \
+ ____cmpxchg(".d", ".d" sc_sfx, prepend, append, \
+ __ret, __rc, __ptr, /**/, __old, __new); \
break; \
default: \
BUILD_BUG(); \
@@ -245,105 +201,37 @@
__ret; \
})
-#define arch_cmpxchg_acquire(ptr, o, n) \
+#define __cmpxchg_relaxed(ptr, old, new, size) \
+ ___cmpxchg(ptr, old, new, size, "", "", "")
+
+#define _arch_cmpxchg(order, ptr, o, n) \
({ \
__typeof__(*(ptr)) _o_ = (o); \
__typeof__(*(ptr)) _n_ = (n); \
- (__typeof__(*(ptr))) __cmpxchg_acquire((ptr), \
- _o_, _n_, sizeof(*(ptr))); \
+ (__typeof__(*(ptr))) __cmpxchg ## order((ptr), _o_, _n_, \
+ sizeof(*(ptr))); \
})
+#define arch_cmpxchg_relaxed(ptr, o, n) \
+ _arch_cmpxchg(_relaxed, ptr, o, n)
+
+#define __cmpxchg_acquire(ptr, old, new, size) \
+ ___cmpxchg(ptr, old, new, size, "", "", RISCV_ACQUIRE_BARRIER)
+
+#define arch_cmpxchg_acquire(ptr, o, n) \
+ _arch_cmpxchg(_acquire, ptr, o, n)
+
#define __cmpxchg_release(ptr, old, new, size) \
-({ \
- __typeof__(ptr) __ptr = (ptr); \
- __typeof__(*(ptr)) __old = (old); \
- __typeof__(*(ptr)) __new = (new); \
- __typeof__(*(ptr)) __ret; \
- register unsigned int __rc; \
- switch (size) { \
- case 4: \
- __asm__ __volatile__ ( \
- RISCV_RELEASE_BARRIER \
- "0: lr.w %0, %2\n" \
- " bne %0, %z3, 1f\n" \
- " sc.w %1, %z4, %2\n" \
- " bnez %1, 0b\n" \
- "1:\n" \
- : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
- : "rJ" ((long)__old), "rJ" (__new) \
- : "memory"); \
- break; \
- case 8: \
- __asm__ __volatile__ ( \
- RISCV_RELEASE_BARRIER \
- "0: lr.d %0, %2\n" \
- " bne %0, %z3, 1f\n" \
- " sc.d %1, %z4, %2\n" \
- " bnez %1, 0b\n" \
- "1:\n" \
- : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
- : "rJ" (__old), "rJ" (__new) \
- : "memory"); \
- break; \
- default: \
- BUILD_BUG(); \
- } \
- __ret; \
-})
+ ___cmpxchg(ptr, old, new, size, "", RISCV_RELEASE_BARRIER, "")
#define arch_cmpxchg_release(ptr, o, n) \
-({ \
- __typeof__(*(ptr)) _o_ = (o); \
- __typeof__(*(ptr)) _n_ = (n); \
- (__typeof__(*(ptr))) __cmpxchg_release((ptr), \
- _o_, _n_, sizeof(*(ptr))); \
-})
+ _arch_cmpxchg(_release, ptr, o, n)
#define __cmpxchg(ptr, old, new, size) \
-({ \
- __typeof__(ptr) __ptr = (ptr); \
- __typeof__(*(ptr)) __old = (old); \
- __typeof__(*(ptr)) __new = (new); \
- __typeof__(*(ptr)) __ret; \
- register unsigned int __rc; \
- switch (size) { \
- case 4: \
- __asm__ __volatile__ ( \
- "0: lr.w %0, %2\n" \
- " bne %0, %z3, 1f\n" \
- " sc.w.rl %1, %z4, %2\n" \
- " bnez %1, 0b\n" \
- " fence rw, rw\n" \
- "1:\n" \
- : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
- : "rJ" ((long)__old), "rJ" (__new) \
- : "memory"); \
- break; \
- case 8: \
- __asm__ __volatile__ ( \
- "0: lr.d %0, %2\n" \
- " bne %0, %z3, 1f\n" \
- " sc.d.rl %1, %z4, %2\n" \
- " bnez %1, 0b\n" \
- " fence rw, rw\n" \
- "1:\n" \
- : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
- : "rJ" (__old), "rJ" (__new) \
- : "memory"); \
- break; \
- default: \
- BUILD_BUG(); \
- } \
- __ret; \
-})
+ ___cmpxchg(ptr, old, new, size, ".rl", "", " fence rw, rw\n")
#define arch_cmpxchg(ptr, o, n) \
-({ \
- __typeof__(*(ptr)) _o_ = (o); \
- __typeof__(*(ptr)) _n_ = (n); \
- (__typeof__(*(ptr))) __cmpxchg((ptr), \
- _o_, _n_, sizeof(*(ptr))); \
-})
+ _arch_cmpxchg(, ptr, o, n)
#define arch_cmpxchg_local(ptr, o, n) \
(__cmpxchg_relaxed((ptr), (o), (n), sizeof(*(ptr))))
--
2.40.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH v1 2/2] riscv/cmpxchg: Deduplicate xchg() asm functions
2023-04-06 8:20 [RFC PATCH v1 0/2] Deduplicating RISCV cmpxchg.h macros Leonardo Bras
2023-04-06 8:20 ` [RFC PATCH v1 1/2] riscv/cmpxchg: Deduplicate cmpxchg() asm and macros Leonardo Bras
@ 2023-04-06 8:20 ` Leonardo Bras
2023-04-07 8:31 ` Guo Ren
2023-04-06 17:19 ` [RFC PATCH v1 0/2] Deduplicating RISCV cmpxchg.h macros Andrea Parri
2 siblings, 1 reply; 10+ messages in thread
From: Leonardo Bras @ 2023-04-06 8:20 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Leonardo Bras, Guo Ren,
Andrea Parri, Conor Dooley
Cc: linux-riscv, linux-kernel
In this header every xchg define (_relaxed, _acquire, _release, vanilla)
contain it's own asm file, both for 4-byte variables an 8-byte variables,
on a total of 8 versions of mostly the same asm.
This is usually bad, as it means any change may be done in up to 8
different places.
Unify those versions by creating a new define with enough parameters to
generate any version of the previous 8.
Then unify the result under a more general define, and simplify
arch_xchg* generation.
(This did not cause any change in generated asm)
Signed-off-by: Leonardo Bras <leobras@redhat.com>
---
arch/riscv/include/asm/cmpxchg.h | 135 +++++++------------------------
1 file changed, 31 insertions(+), 104 deletions(-)
diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
index f88fae357071c..905a888d8b04d 100644
--- a/arch/riscv/include/asm/cmpxchg.h
+++ b/arch/riscv/include/asm/cmpxchg.h
@@ -11,25 +11,30 @@
#include <asm/barrier.h>
#include <asm/fence.h>
-#define __xchg_relaxed(ptr, new, size) \
+#define ____xchg(sfx, prepend, append, r, p, n) \
+({ \
+ __asm__ __volatile__ ( \
+ prepend \
+ " amoswap" sfx " %0, %2, %1\n" \
+ append \
+ : "=r" (r), "+A" (*(p)) \
+ : "r" (n) \
+ : "memory"); \
+})
+
+#define ___xchg(ptr, new, size, sfx, prepend, append) \
({ \
__typeof__(ptr) __ptr = (ptr); \
__typeof__(new) __new = (new); \
__typeof__(*(ptr)) __ret; \
switch (size) { \
case 4: \
- __asm__ __volatile__ ( \
- " amoswap.w %0, %2, %1\n" \
- : "=r" (__ret), "+A" (*__ptr) \
- : "r" (__new) \
- : "memory"); \
+ ____xchg(".w" sfx, prepend, append, \
+ __ret, __ptr, __new); \
break; \
case 8: \
- __asm__ __volatile__ ( \
- " amoswap.d %0, %2, %1\n" \
- : "=r" (__ret), "+A" (*__ptr) \
- : "r" (__new) \
- : "memory"); \
+ ____xchg(".d" sfx, prepend, append, \
+ __ret, __ptr, __new); \
break; \
default: \
BUILD_BUG(); \
@@ -37,114 +42,36 @@
__ret; \
})
-#define arch_xchg_relaxed(ptr, x) \
+#define __xchg_relaxed(ptr, new, size) \
+ ___xchg(ptr, new, size, "", "", "")
+
+#define _arch_xchg(order, ptr, x) \
({ \
__typeof__(*(ptr)) _x_ = (x); \
- (__typeof__(*(ptr))) __xchg_relaxed((ptr), \
- _x_, sizeof(*(ptr))); \
+ (__typeof__(*(ptr))) __xchg ## order((ptr), \
+ _x_, sizeof(*(ptr))); \
})
+#define arch_xchg_relaxed(ptr, x) \
+ _arch_xchg(_relaxed, ptr, x)
+
#define __xchg_acquire(ptr, new, size) \
-({ \
- __typeof__(ptr) __ptr = (ptr); \
- __typeof__(new) __new = (new); \
- __typeof__(*(ptr)) __ret; \
- switch (size) { \
- case 4: \
- __asm__ __volatile__ ( \
- " amoswap.w %0, %2, %1\n" \
- RISCV_ACQUIRE_BARRIER \
- : "=r" (__ret), "+A" (*__ptr) \
- : "r" (__new) \
- : "memory"); \
- break; \
- case 8: \
- __asm__ __volatile__ ( \
- " amoswap.d %0, %2, %1\n" \
- RISCV_ACQUIRE_BARRIER \
- : "=r" (__ret), "+A" (*__ptr) \
- : "r" (__new) \
- : "memory"); \
- break; \
- default: \
- BUILD_BUG(); \
- } \
- __ret; \
-})
+ ___xchg(ptr, new, size, "", "", RISCV_ACQUIRE_BARRIER)
#define arch_xchg_acquire(ptr, x) \
-({ \
- __typeof__(*(ptr)) _x_ = (x); \
- (__typeof__(*(ptr))) __xchg_acquire((ptr), \
- _x_, sizeof(*(ptr))); \
-})
+ _arch_xchg(_acquire, ptr, x)
#define __xchg_release(ptr, new, size) \
-({ \
- __typeof__(ptr) __ptr = (ptr); \
- __typeof__(new) __new = (new); \
- __typeof__(*(ptr)) __ret; \
- switch (size) { \
- case 4: \
- __asm__ __volatile__ ( \
- RISCV_RELEASE_BARRIER \
- " amoswap.w %0, %2, %1\n" \
- : "=r" (__ret), "+A" (*__ptr) \
- : "r" (__new) \
- : "memory"); \
- break; \
- case 8: \
- __asm__ __volatile__ ( \
- RISCV_RELEASE_BARRIER \
- " amoswap.d %0, %2, %1\n" \
- : "=r" (__ret), "+A" (*__ptr) \
- : "r" (__new) \
- : "memory"); \
- break; \
- default: \
- BUILD_BUG(); \
- } \
- __ret; \
-})
+ ___xchg(ptr, new, size, "", RISCV_RELEASE_BARRIER, "")
#define arch_xchg_release(ptr, x) \
-({ \
- __typeof__(*(ptr)) _x_ = (x); \
- (__typeof__(*(ptr))) __xchg_release((ptr), \
- _x_, sizeof(*(ptr))); \
-})
+ _arch_xchg(_release, ptr, x)
#define __xchg(ptr, new, size) \
-({ \
- __typeof__(ptr) __ptr = (ptr); \
- __typeof__(new) __new = (new); \
- __typeof__(*(ptr)) __ret; \
- switch (size) { \
- case 4: \
- __asm__ __volatile__ ( \
- " amoswap.w.aqrl %0, %2, %1\n" \
- : "=r" (__ret), "+A" (*__ptr) \
- : "r" (__new) \
- : "memory"); \
- break; \
- case 8: \
- __asm__ __volatile__ ( \
- " amoswap.d.aqrl %0, %2, %1\n" \
- : "=r" (__ret), "+A" (*__ptr) \
- : "r" (__new) \
- : "memory"); \
- break; \
- default: \
- BUILD_BUG(); \
- } \
- __ret; \
-})
+ ___xchg(ptr, new, size, ".aqrl", "", "")
#define arch_xchg(ptr, x) \
-({ \
- __typeof__(*(ptr)) _x_ = (x); \
- (__typeof__(*(ptr))) __xchg((ptr), _x_, sizeof(*(ptr))); \
-})
+ _arch_xchg(, ptr, x)
#define xchg32(ptr, x) \
({ \
--
2.40.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v1 0/2] Deduplicating RISCV cmpxchg.h macros
2023-04-06 8:20 [RFC PATCH v1 0/2] Deduplicating RISCV cmpxchg.h macros Leonardo Bras
2023-04-06 8:20 ` [RFC PATCH v1 1/2] riscv/cmpxchg: Deduplicate cmpxchg() asm and macros Leonardo Bras
2023-04-06 8:20 ` [RFC PATCH v1 2/2] riscv/cmpxchg: Deduplicate xchg() asm functions Leonardo Bras
@ 2023-04-06 17:19 ` Andrea Parri
2023-08-02 21:51 ` Leonardo Brás
2 siblings, 1 reply; 10+ messages in thread
From: Andrea Parri @ 2023-04-06 17:19 UTC (permalink / raw)
To: Leonardo Bras
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Guo Ren, Conor Dooley,
linux-riscv, linux-kernel
On Thu, Apr 06, 2023 at 05:20:17AM -0300, Leonardo Bras wrote:
> While studying riscv's cmpxchg.h file, I got really interested in
> understanding how RISCV asm implemented the different versions of
> {cmp,}xchg.
>
> When I understood the pattern, it made sense for me to remove the
> duplications and create macros to make it easier to understand what exactly
> changes between the versions: Instruction sufixes & barriers.
>
> Thanks!
> Leo
>
> Changes since RFCv3:
> - Squashed the 6 original patches in 2: one for cmpxchg and one for xchg
> https://lore.kernel.org/all/20230404163741.2762165-1-leobras@redhat.com/
>
> Changes since RFCv2:
> - Fixed macros that depend on having a local variable with a magic name
> - Previous cast to (long) is now only applied on 4-bytes cmpxchg
> https://lore.kernel.org/all/20230321074249.2221674-1-leobras@redhat.com/
>
> Changes since RFCv1:
> - Fixed patch 4/6 suffix from 'w.aqrl' to '.w.aqrl', to avoid build error
> https://lore.kernel.org/all/20230318080059.1109286-1-leobras@redhat.com/
>
> Leonardo Bras (2):
> riscv/cmpxchg: Deduplicate cmpxchg() asm and macros
> riscv/cmpxchg: Deduplicate xchg() asm functions
>
> arch/riscv/include/asm/cmpxchg.h | 319 +++++++------------------------
> 1 file changed, 67 insertions(+), 252 deletions(-)
LGTM. AFAICT, this would need to be rebased, cf. e.g.
a8596dda1fbf7e ("arch: rename all internal names __xchg to __arch_xchg")
from the tip tree.
Andrea
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v1 1/2] riscv/cmpxchg: Deduplicate cmpxchg() asm and macros
2023-04-06 8:20 ` [RFC PATCH v1 1/2] riscv/cmpxchg: Deduplicate cmpxchg() asm and macros Leonardo Bras
@ 2023-04-07 8:29 ` Guo Ren
2023-04-18 19:10 ` Leonardo Bras Soares Passos
0 siblings, 1 reply; 10+ messages in thread
From: Guo Ren @ 2023-04-07 8:29 UTC (permalink / raw)
To: Leonardo Bras
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrea Parri,
Conor Dooley, linux-riscv, linux-kernel
On Thu, Apr 6, 2023 at 4:20 PM Leonardo Bras <leobras@redhat.com> wrote:
>
> In this header every cmpxchg define (_relaxed, _acquire, _release,
> vanilla) contain it's own asm file, both for 4-byte variables an 8-byte
> variables, on a total of 8 versions of mostly the same asm.
>
> This is usually bad, as it means any change may be done in up to 8
> different places.
>
> Unify those versions by creating a new define with enough parameters to
> generate any version of the previous 8.
>
> Then unify the result under a more general define, and simplify
> arch_cmpxchg* generation
>
> (This did not cause any change in generated asm)
>
> Signed-off-by: Leonardo Bras <leobras@redhat.com>
> ---
> arch/riscv/include/asm/cmpxchg.h | 184 ++++++-------------------------
> 1 file changed, 36 insertions(+), 148 deletions(-)
>
> diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> index 12debce235e52..f88fae357071c 100644
> --- a/arch/riscv/include/asm/cmpxchg.h
> +++ b/arch/riscv/include/asm/cmpxchg.h
> @@ -163,51 +163,23 @@
> * store NEW in MEM. Return the initial value in MEM. Success is
> * indicated by comparing RETURN with OLD.
> */
> -#define __cmpxchg_relaxed(ptr, old, new, size) \
> -({ \
> - __typeof__(ptr) __ptr = (ptr); \
> - __typeof__(*(ptr)) __old = (old); \
> - __typeof__(*(ptr)) __new = (new); \
> - __typeof__(*(ptr)) __ret; \
> - register unsigned int __rc; \
> - switch (size) { \
> - case 4: \
> - __asm__ __volatile__ ( \
> - "0: lr.w %0, %2\n" \
> - " bne %0, %z3, 1f\n" \
> - " sc.w %1, %z4, %2\n" \
> - " bnez %1, 0b\n" \
> - "1:\n" \
> - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> - : "rJ" ((long)__old), "rJ" (__new) \
> - : "memory"); \
> - break; \
> - case 8: \
> - __asm__ __volatile__ ( \
> - "0: lr.d %0, %2\n" \
> - " bne %0, %z3, 1f\n" \
> - " sc.d %1, %z4, %2\n" \
> - " bnez %1, 0b\n" \
> - "1:\n" \
> - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> - : "rJ" (__old), "rJ" (__new) \
> - : "memory"); \
> - break; \
> - default: \
> - BUILD_BUG(); \
> - } \
> - __ret; \
> -})
>
> -#define arch_cmpxchg_relaxed(ptr, o, n) \
> +#define ____cmpxchg(lr_sfx, sc_sfx, prepend, append, r, rc, p, co, o, n)\
> ({ \
> - __typeof__(*(ptr)) _o_ = (o); \
> - __typeof__(*(ptr)) _n_ = (n); \
> - (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr), \
> - _o_, _n_, sizeof(*(ptr))); \
> + __asm__ __volatile__ ( \
> + prepend \
> + "0: lr" lr_sfx " %0, %2\n" \
> + " bne %0, %z3, 1f\n" \
> + " sc" sc_sfx " %1, %z4, %2\n" \
> + " bnez %1, 0b\n" \
> + append \
> + "1:\n" \
> + : "=&r" (r), "=&r" (rc), "+A" (*(p)) \
> + : "rJ" (co o), "rJ" (n) \
> + : "memory"); \
> })
>
> -#define __cmpxchg_acquire(ptr, old, new, size) \
> +#define ___cmpxchg(ptr, old, new, size, sc_sfx, prepend, append) \
> ({ \
> __typeof__(ptr) __ptr = (ptr); \
> __typeof__(*(ptr)) __old = (old); \
> @@ -216,28 +188,12 @@
> register unsigned int __rc; \
> switch (size) { \
> case 4: \
> - __asm__ __volatile__ ( \
> - "0: lr.w %0, %2\n" \
> - " bne %0, %z3, 1f\n" \
> - " sc.w %1, %z4, %2\n" \
> - " bnez %1, 0b\n" \
> - RISCV_ACQUIRE_BARRIER \
> - "1:\n" \
> - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> - : "rJ" ((long)__old), "rJ" (__new) \
> - : "memory"); \
> + ____cmpxchg(".w", ".w" sc_sfx, prepend, append, \
> + __ret, __rc, __ptr, (long), __old, __new); \
> break; \
> case 8: \
> - __asm__ __volatile__ ( \
> - "0: lr.d %0, %2\n" \
> - " bne %0, %z3, 1f\n" \
> - " sc.d %1, %z4, %2\n" \
> - " bnez %1, 0b\n" \
> - RISCV_ACQUIRE_BARRIER \
> - "1:\n" \
> - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> - : "rJ" (__old), "rJ" (__new) \
> - : "memory"); \
> + ____cmpxchg(".d", ".d" sc_sfx, prepend, append, \
> + __ret, __rc, __ptr, /**/, __old, __new); \
> break; \
> default: \
> BUILD_BUG(); \
> @@ -245,105 +201,37 @@
> __ret; \
> })
>
> -#define arch_cmpxchg_acquire(ptr, o, n) \
> +#define __cmpxchg_relaxed(ptr, old, new, size) \
> + ___cmpxchg(ptr, old, new, size, "", "", "")
> +
> +#define _arch_cmpxchg(order, ptr, o, n) \
> ({ \
> __typeof__(*(ptr)) _o_ = (o); \
> __typeof__(*(ptr)) _n_ = (n); \
> - (__typeof__(*(ptr))) __cmpxchg_acquire((ptr), \
> - _o_, _n_, sizeof(*(ptr))); \
> + (__typeof__(*(ptr))) __cmpxchg ## order((ptr), _o_, _n_, \
> + sizeof(*(ptr))); \
> })
>
> +#define arch_cmpxchg_relaxed(ptr, o, n) \
> + _arch_cmpxchg(_relaxed, ptr, o, n)
> +
> +#define __cmpxchg_acquire(ptr, old, new, size) \
> + ___cmpxchg(ptr, old, new, size, "", "", RISCV_ACQUIRE_BARRIER)
> +
> +#define arch_cmpxchg_acquire(ptr, o, n) \
> + _arch_cmpxchg(_acquire, ptr, o, n)
> +
> #define __cmpxchg_release(ptr, old, new, size) \
> -({ \
> - __typeof__(ptr) __ptr = (ptr); \
> - __typeof__(*(ptr)) __old = (old); \
> - __typeof__(*(ptr)) __new = (new); \
> - __typeof__(*(ptr)) __ret; \
> - register unsigned int __rc; \
> - switch (size) { \
> - case 4: \
> - __asm__ __volatile__ ( \
> - RISCV_RELEASE_BARRIER \
> - "0: lr.w %0, %2\n" \
> - " bne %0, %z3, 1f\n" \
> - " sc.w %1, %z4, %2\n" \
> - " bnez %1, 0b\n" \
> - "1:\n" \
> - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> - : "rJ" ((long)__old), "rJ" (__new) \
> - : "memory"); \
> - break; \
> - case 8: \
> - __asm__ __volatile__ ( \
> - RISCV_RELEASE_BARRIER \
> - "0: lr.d %0, %2\n" \
> - " bne %0, %z3, 1f\n" \
> - " sc.d %1, %z4, %2\n" \
> - " bnez %1, 0b\n" \
> - "1:\n" \
> - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> - : "rJ" (__old), "rJ" (__new) \
> - : "memory"); \
> - break; \
> - default: \
> - BUILD_BUG(); \
> - } \
> - __ret; \
> -})
> + ___cmpxchg(ptr, old, new, size, "", RISCV_RELEASE_BARRIER, "")
>
> #define arch_cmpxchg_release(ptr, o, n) \
> -({ \
> - __typeof__(*(ptr)) _o_ = (o); \
> - __typeof__(*(ptr)) _n_ = (n); \
> - (__typeof__(*(ptr))) __cmpxchg_release((ptr), \
> - _o_, _n_, sizeof(*(ptr))); \
> -})
> + _arch_cmpxchg(_release, ptr, o, n)
>
> #define __cmpxchg(ptr, old, new, size) \
> -({ \
> - __typeof__(ptr) __ptr = (ptr); \
> - __typeof__(*(ptr)) __old = (old); \
> - __typeof__(*(ptr)) __new = (new); \
> - __typeof__(*(ptr)) __ret; \
> - register unsigned int __rc; \
> - switch (size) { \
> - case 4: \
> - __asm__ __volatile__ ( \
> - "0: lr.w %0, %2\n" \
> - " bne %0, %z3, 1f\n" \
> - " sc.w.rl %1, %z4, %2\n" \
> - " bnez %1, 0b\n" \
> - " fence rw, rw\n" \
> - "1:\n" \
> - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> - : "rJ" ((long)__old), "rJ" (__new) \
> - : "memory"); \
> - break; \
> - case 8: \
> - __asm__ __volatile__ ( \
> - "0: lr.d %0, %2\n" \
> - " bne %0, %z3, 1f\n" \
> - " sc.d.rl %1, %z4, %2\n" \
> - " bnez %1, 0b\n" \
> - " fence rw, rw\n" \
> - "1:\n" \
> - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> - : "rJ" (__old), "rJ" (__new) \
> - : "memory"); \
> - break; \
> - default: \
> - BUILD_BUG(); \
> - } \
> - __ret; \
> -})
> + ___cmpxchg(ptr, old, new, size, ".rl", "", " fence rw, rw\n")
>
> #define arch_cmpxchg(ptr, o, n) \
> -({ \
> - __typeof__(*(ptr)) _o_ = (o); \
> - __typeof__(*(ptr)) _n_ = (n); \
> - (__typeof__(*(ptr))) __cmpxchg((ptr), \
> - _o_, _n_, sizeof(*(ptr))); \
> -})
> + _arch_cmpxchg(, ptr, o, n)
>
> #define arch_cmpxchg_local(ptr, o, n) \
> (__cmpxchg_relaxed((ptr), (o), (n), sizeof(*(ptr))))
> --
> 2.40.0
>
One patch is much easier to review :)
Reviewed-by: Guo Ren <guoren@kernel.org>
--
Best Regards
Guo Ren
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v1 2/2] riscv/cmpxchg: Deduplicate xchg() asm functions
2023-04-06 8:20 ` [RFC PATCH v1 2/2] riscv/cmpxchg: Deduplicate xchg() asm functions Leonardo Bras
@ 2023-04-07 8:31 ` Guo Ren
0 siblings, 0 replies; 10+ messages in thread
From: Guo Ren @ 2023-04-07 8:31 UTC (permalink / raw)
To: Leonardo Bras
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrea Parri,
Conor Dooley, linux-riscv, linux-kernel
On Thu, Apr 6, 2023 at 4:20 PM Leonardo Bras <leobras@redhat.com> wrote:
>
> In this header every xchg define (_relaxed, _acquire, _release, vanilla)
> contain it's own asm file, both for 4-byte variables an 8-byte variables,
> on a total of 8 versions of mostly the same asm.
>
> This is usually bad, as it means any change may be done in up to 8
> different places.
>
> Unify those versions by creating a new define with enough parameters to
> generate any version of the previous 8.
>
> Then unify the result under a more general define, and simplify
> arch_xchg* generation.
>
> (This did not cause any change in generated asm)
>
> Signed-off-by: Leonardo Bras <leobras@redhat.com>
> ---
> arch/riscv/include/asm/cmpxchg.h | 135 +++++++------------------------
> 1 file changed, 31 insertions(+), 104 deletions(-)
>
> diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> index f88fae357071c..905a888d8b04d 100644
> --- a/arch/riscv/include/asm/cmpxchg.h
> +++ b/arch/riscv/include/asm/cmpxchg.h
> @@ -11,25 +11,30 @@
> #include <asm/barrier.h>
> #include <asm/fence.h>
>
> -#define __xchg_relaxed(ptr, new, size) \
> +#define ____xchg(sfx, prepend, append, r, p, n) \
> +({ \
> + __asm__ __volatile__ ( \
> + prepend \
> + " amoswap" sfx " %0, %2, %1\n" \
> + append \
> + : "=r" (r), "+A" (*(p)) \
> + : "r" (n) \
> + : "memory"); \
> +})
> +
> +#define ___xchg(ptr, new, size, sfx, prepend, append) \
> ({ \
> __typeof__(ptr) __ptr = (ptr); \
> __typeof__(new) __new = (new); \
> __typeof__(*(ptr)) __ret; \
> switch (size) { \
> case 4: \
> - __asm__ __volatile__ ( \
> - " amoswap.w %0, %2, %1\n" \
> - : "=r" (__ret), "+A" (*__ptr) \
> - : "r" (__new) \
> - : "memory"); \
> + ____xchg(".w" sfx, prepend, append, \
> + __ret, __ptr, __new); \
> break; \
> case 8: \
> - __asm__ __volatile__ ( \
> - " amoswap.d %0, %2, %1\n" \
> - : "=r" (__ret), "+A" (*__ptr) \
> - : "r" (__new) \
> - : "memory"); \
> + ____xchg(".d" sfx, prepend, append, \
> + __ret, __ptr, __new); \
> break; \
> default: \
> BUILD_BUG(); \
> @@ -37,114 +42,36 @@
> __ret; \
> })
>
> -#define arch_xchg_relaxed(ptr, x) \
> +#define __xchg_relaxed(ptr, new, size) \
> + ___xchg(ptr, new, size, "", "", "")
> +
> +#define _arch_xchg(order, ptr, x) \
> ({ \
> __typeof__(*(ptr)) _x_ = (x); \
> - (__typeof__(*(ptr))) __xchg_relaxed((ptr), \
> - _x_, sizeof(*(ptr))); \
> + (__typeof__(*(ptr))) __xchg ## order((ptr), \
> + _x_, sizeof(*(ptr))); \
> })
>
> +#define arch_xchg_relaxed(ptr, x) \
> + _arch_xchg(_relaxed, ptr, x)
> +
> #define __xchg_acquire(ptr, new, size) \
> -({ \
> - __typeof__(ptr) __ptr = (ptr); \
> - __typeof__(new) __new = (new); \
> - __typeof__(*(ptr)) __ret; \
> - switch (size) { \
> - case 4: \
> - __asm__ __volatile__ ( \
> - " amoswap.w %0, %2, %1\n" \
> - RISCV_ACQUIRE_BARRIER \
> - : "=r" (__ret), "+A" (*__ptr) \
> - : "r" (__new) \
> - : "memory"); \
> - break; \
> - case 8: \
> - __asm__ __volatile__ ( \
> - " amoswap.d %0, %2, %1\n" \
> - RISCV_ACQUIRE_BARRIER \
> - : "=r" (__ret), "+A" (*__ptr) \
> - : "r" (__new) \
> - : "memory"); \
> - break; \
> - default: \
> - BUILD_BUG(); \
> - } \
> - __ret; \
> -})
> + ___xchg(ptr, new, size, "", "", RISCV_ACQUIRE_BARRIER)
>
> #define arch_xchg_acquire(ptr, x) \
> -({ \
> - __typeof__(*(ptr)) _x_ = (x); \
> - (__typeof__(*(ptr))) __xchg_acquire((ptr), \
> - _x_, sizeof(*(ptr))); \
> -})
> + _arch_xchg(_acquire, ptr, x)
>
> #define __xchg_release(ptr, new, size) \
> -({ \
> - __typeof__(ptr) __ptr = (ptr); \
> - __typeof__(new) __new = (new); \
> - __typeof__(*(ptr)) __ret; \
> - switch (size) { \
> - case 4: \
> - __asm__ __volatile__ ( \
> - RISCV_RELEASE_BARRIER \
> - " amoswap.w %0, %2, %1\n" \
> - : "=r" (__ret), "+A" (*__ptr) \
> - : "r" (__new) \
> - : "memory"); \
> - break; \
> - case 8: \
> - __asm__ __volatile__ ( \
> - RISCV_RELEASE_BARRIER \
> - " amoswap.d %0, %2, %1\n" \
> - : "=r" (__ret), "+A" (*__ptr) \
> - : "r" (__new) \
> - : "memory"); \
> - break; \
> - default: \
> - BUILD_BUG(); \
> - } \
> - __ret; \
> -})
> + ___xchg(ptr, new, size, "", RISCV_RELEASE_BARRIER, "")
>
> #define arch_xchg_release(ptr, x) \
> -({ \
> - __typeof__(*(ptr)) _x_ = (x); \
> - (__typeof__(*(ptr))) __xchg_release((ptr), \
> - _x_, sizeof(*(ptr))); \
> -})
> + _arch_xchg(_release, ptr, x)
>
> #define __xchg(ptr, new, size) \
> -({ \
> - __typeof__(ptr) __ptr = (ptr); \
> - __typeof__(new) __new = (new); \
> - __typeof__(*(ptr)) __ret; \
> - switch (size) { \
> - case 4: \
> - __asm__ __volatile__ ( \
> - " amoswap.w.aqrl %0, %2, %1\n" \
> - : "=r" (__ret), "+A" (*__ptr) \
> - : "r" (__new) \
> - : "memory"); \
> - break; \
> - case 8: \
> - __asm__ __volatile__ ( \
> - " amoswap.d.aqrl %0, %2, %1\n" \
> - : "=r" (__ret), "+A" (*__ptr) \
> - : "r" (__new) \
> - : "memory"); \
> - break; \
> - default: \
> - BUILD_BUG(); \
> - } \
> - __ret; \
> -})
> + ___xchg(ptr, new, size, ".aqrl", "", "")
>
> #define arch_xchg(ptr, x) \
> -({ \
> - __typeof__(*(ptr)) _x_ = (x); \
> - (__typeof__(*(ptr))) __xchg((ptr), _x_, sizeof(*(ptr))); \
> -})
> + _arch_xchg(, ptr, x)
>
> #define xchg32(ptr, x) \
> ({ \
> --
> 2.40.0
>
Reviewed-by: Guo Ren <guoren@kernel.org>
--
Best Regards
Guo Ren
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v1 1/2] riscv/cmpxchg: Deduplicate cmpxchg() asm and macros
2023-04-07 8:29 ` Guo Ren
@ 2023-04-18 19:10 ` Leonardo Bras Soares Passos
0 siblings, 0 replies; 10+ messages in thread
From: Leonardo Bras Soares Passos @ 2023-04-18 19:10 UTC (permalink / raw)
To: Guo Ren
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrea Parri,
Conor Dooley, linux-riscv, linux-kernel
On Fri, Apr 7, 2023 at 5:29 AM Guo Ren <guoren@kernel.org> wrote:
>
> On Thu, Apr 6, 2023 at 4:20 PM Leonardo Bras <leobras@redhat.com> wrote:
> >
> > In this header every cmpxchg define (_relaxed, _acquire, _release,
> > vanilla) contain it's own asm file, both for 4-byte variables an 8-byte
> > variables, on a total of 8 versions of mostly the same asm.
> >
> > This is usually bad, as it means any change may be done in up to 8
> > different places.
> >
> > Unify those versions by creating a new define with enough parameters to
> > generate any version of the previous 8.
> >
> > Then unify the result under a more general define, and simplify
> > arch_cmpxchg* generation
> >
> > (This did not cause any change in generated asm)
> >
> > Signed-off-by: Leonardo Bras <leobras@redhat.com>
> > ---
> > arch/riscv/include/asm/cmpxchg.h | 184 ++++++-------------------------
> > 1 file changed, 36 insertions(+), 148 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> > index 12debce235e52..f88fae357071c 100644
> > --- a/arch/riscv/include/asm/cmpxchg.h
> > +++ b/arch/riscv/include/asm/cmpxchg.h
> > @@ -163,51 +163,23 @@
> > * store NEW in MEM. Return the initial value in MEM. Success is
> > * indicated by comparing RETURN with OLD.
> > */
> > -#define __cmpxchg_relaxed(ptr, old, new, size) \
> > -({ \
> > - __typeof__(ptr) __ptr = (ptr); \
> > - __typeof__(*(ptr)) __old = (old); \
> > - __typeof__(*(ptr)) __new = (new); \
> > - __typeof__(*(ptr)) __ret; \
> > - register unsigned int __rc; \
> > - switch (size) { \
> > - case 4: \
> > - __asm__ __volatile__ ( \
> > - "0: lr.w %0, %2\n" \
> > - " bne %0, %z3, 1f\n" \
> > - " sc.w %1, %z4, %2\n" \
> > - " bnez %1, 0b\n" \
> > - "1:\n" \
> > - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> > - : "rJ" ((long)__old), "rJ" (__new) \
> > - : "memory"); \
> > - break; \
> > - case 8: \
> > - __asm__ __volatile__ ( \
> > - "0: lr.d %0, %2\n" \
> > - " bne %0, %z3, 1f\n" \
> > - " sc.d %1, %z4, %2\n" \
> > - " bnez %1, 0b\n" \
> > - "1:\n" \
> > - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> > - : "rJ" (__old), "rJ" (__new) \
> > - : "memory"); \
> > - break; \
> > - default: \
> > - BUILD_BUG(); \
> > - } \
> > - __ret; \
> > -})
> >
> > -#define arch_cmpxchg_relaxed(ptr, o, n) \
> > +#define ____cmpxchg(lr_sfx, sc_sfx, prepend, append, r, rc, p, co, o, n)\
> > ({ \
> > - __typeof__(*(ptr)) _o_ = (o); \
> > - __typeof__(*(ptr)) _n_ = (n); \
> > - (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr), \
> > - _o_, _n_, sizeof(*(ptr))); \
> > + __asm__ __volatile__ ( \
> > + prepend \
> > + "0: lr" lr_sfx " %0, %2\n" \
> > + " bne %0, %z3, 1f\n" \
> > + " sc" sc_sfx " %1, %z4, %2\n" \
> > + " bnez %1, 0b\n" \
> > + append \
> > + "1:\n" \
> > + : "=&r" (r), "=&r" (rc), "+A" (*(p)) \
> > + : "rJ" (co o), "rJ" (n) \
> > + : "memory"); \
> > })
> >
> > -#define __cmpxchg_acquire(ptr, old, new, size) \
> > +#define ___cmpxchg(ptr, old, new, size, sc_sfx, prepend, append) \
> > ({ \
> > __typeof__(ptr) __ptr = (ptr); \
> > __typeof__(*(ptr)) __old = (old); \
> > @@ -216,28 +188,12 @@
> > register unsigned int __rc; \
> > switch (size) { \
> > case 4: \
> > - __asm__ __volatile__ ( \
> > - "0: lr.w %0, %2\n" \
> > - " bne %0, %z3, 1f\n" \
> > - " sc.w %1, %z4, %2\n" \
> > - " bnez %1, 0b\n" \
> > - RISCV_ACQUIRE_BARRIER \
> > - "1:\n" \
> > - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> > - : "rJ" ((long)__old), "rJ" (__new) \
> > - : "memory"); \
> > + ____cmpxchg(".w", ".w" sc_sfx, prepend, append, \
> > + __ret, __rc, __ptr, (long), __old, __new); \
> > break; \
> > case 8: \
> > - __asm__ __volatile__ ( \
> > - "0: lr.d %0, %2\n" \
> > - " bne %0, %z3, 1f\n" \
> > - " sc.d %1, %z4, %2\n" \
> > - " bnez %1, 0b\n" \
> > - RISCV_ACQUIRE_BARRIER \
> > - "1:\n" \
> > - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> > - : "rJ" (__old), "rJ" (__new) \
> > - : "memory"); \
> > + ____cmpxchg(".d", ".d" sc_sfx, prepend, append, \
> > + __ret, __rc, __ptr, /**/, __old, __new); \
> > break; \
> > default: \
> > BUILD_BUG(); \
> > @@ -245,105 +201,37 @@
> > __ret; \
> > })
> >
> > -#define arch_cmpxchg_acquire(ptr, o, n) \
> > +#define __cmpxchg_relaxed(ptr, old, new, size) \
> > + ___cmpxchg(ptr, old, new, size, "", "", "")
> > +
> > +#define _arch_cmpxchg(order, ptr, o, n) \
> > ({ \
> > __typeof__(*(ptr)) _o_ = (o); \
> > __typeof__(*(ptr)) _n_ = (n); \
> > - (__typeof__(*(ptr))) __cmpxchg_acquire((ptr), \
> > - _o_, _n_, sizeof(*(ptr))); \
> > + (__typeof__(*(ptr))) __cmpxchg ## order((ptr), _o_, _n_, \
> > + sizeof(*(ptr))); \
> > })
> >
> > +#define arch_cmpxchg_relaxed(ptr, o, n) \
> > + _arch_cmpxchg(_relaxed, ptr, o, n)
> > +
> > +#define __cmpxchg_acquire(ptr, old, new, size) \
> > + ___cmpxchg(ptr, old, new, size, "", "", RISCV_ACQUIRE_BARRIER)
> > +
> > +#define arch_cmpxchg_acquire(ptr, o, n) \
> > + _arch_cmpxchg(_acquire, ptr, o, n)
> > +
> > #define __cmpxchg_release(ptr, old, new, size) \
> > -({ \
> > - __typeof__(ptr) __ptr = (ptr); \
> > - __typeof__(*(ptr)) __old = (old); \
> > - __typeof__(*(ptr)) __new = (new); \
> > - __typeof__(*(ptr)) __ret; \
> > - register unsigned int __rc; \
> > - switch (size) { \
> > - case 4: \
> > - __asm__ __volatile__ ( \
> > - RISCV_RELEASE_BARRIER \
> > - "0: lr.w %0, %2\n" \
> > - " bne %0, %z3, 1f\n" \
> > - " sc.w %1, %z4, %2\n" \
> > - " bnez %1, 0b\n" \
> > - "1:\n" \
> > - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> > - : "rJ" ((long)__old), "rJ" (__new) \
> > - : "memory"); \
> > - break; \
> > - case 8: \
> > - __asm__ __volatile__ ( \
> > - RISCV_RELEASE_BARRIER \
> > - "0: lr.d %0, %2\n" \
> > - " bne %0, %z3, 1f\n" \
> > - " sc.d %1, %z4, %2\n" \
> > - " bnez %1, 0b\n" \
> > - "1:\n" \
> > - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> > - : "rJ" (__old), "rJ" (__new) \
> > - : "memory"); \
> > - break; \
> > - default: \
> > - BUILD_BUG(); \
> > - } \
> > - __ret; \
> > -})
> > + ___cmpxchg(ptr, old, new, size, "", RISCV_RELEASE_BARRIER, "")
> >
> > #define arch_cmpxchg_release(ptr, o, n) \
> > -({ \
> > - __typeof__(*(ptr)) _o_ = (o); \
> > - __typeof__(*(ptr)) _n_ = (n); \
> > - (__typeof__(*(ptr))) __cmpxchg_release((ptr), \
> > - _o_, _n_, sizeof(*(ptr))); \
> > -})
> > + _arch_cmpxchg(_release, ptr, o, n)
> >
> > #define __cmpxchg(ptr, old, new, size) \
> > -({ \
> > - __typeof__(ptr) __ptr = (ptr); \
> > - __typeof__(*(ptr)) __old = (old); \
> > - __typeof__(*(ptr)) __new = (new); \
> > - __typeof__(*(ptr)) __ret; \
> > - register unsigned int __rc; \
> > - switch (size) { \
> > - case 4: \
> > - __asm__ __volatile__ ( \
> > - "0: lr.w %0, %2\n" \
> > - " bne %0, %z3, 1f\n" \
> > - " sc.w.rl %1, %z4, %2\n" \
> > - " bnez %1, 0b\n" \
> > - " fence rw, rw\n" \
> > - "1:\n" \
> > - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> > - : "rJ" ((long)__old), "rJ" (__new) \
> > - : "memory"); \
> > - break; \
> > - case 8: \
> > - __asm__ __volatile__ ( \
> > - "0: lr.d %0, %2\n" \
> > - " bne %0, %z3, 1f\n" \
> > - " sc.d.rl %1, %z4, %2\n" \
> > - " bnez %1, 0b\n" \
> > - " fence rw, rw\n" \
> > - "1:\n" \
> > - : "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr) \
> > - : "rJ" (__old), "rJ" (__new) \
> > - : "memory"); \
> > - break; \
> > - default: \
> > - BUILD_BUG(); \
> > - } \
> > - __ret; \
> > -})
> > + ___cmpxchg(ptr, old, new, size, ".rl", "", " fence rw, rw\n")
> >
> > #define arch_cmpxchg(ptr, o, n) \
> > -({ \
> > - __typeof__(*(ptr)) _o_ = (o); \
> > - __typeof__(*(ptr)) _n_ = (n); \
> > - (__typeof__(*(ptr))) __cmpxchg((ptr), \
> > - _o_, _n_, sizeof(*(ptr))); \
> > -})
> > + _arch_cmpxchg(, ptr, o, n)
> >
> > #define arch_cmpxchg_local(ptr, o, n) \
> > (__cmpxchg_relaxed((ptr), (o), (n), sizeof(*(ptr))))
> > --
> > 2.40.0
> >
> One patch is much easier to review :)
>
> Reviewed-by: Guo Ren <guoren@kernel.org>
>
> --
> Best Regards
> Guo Ren
>
Thanks!
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v1 0/2] Deduplicating RISCV cmpxchg.h macros
2023-04-06 17:19 ` [RFC PATCH v1 0/2] Deduplicating RISCV cmpxchg.h macros Andrea Parri
@ 2023-08-02 21:51 ` Leonardo Brás
2023-08-03 3:14 ` Andrea Parri
0 siblings, 1 reply; 10+ messages in thread
From: Leonardo Brás @ 2023-08-02 21:51 UTC (permalink / raw)
To: Andrea Parri
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Guo Ren, Conor Dooley,
linux-riscv, linux-kernel
On Thu, 2023-04-06 at 19:19 +0200, Andrea Parri wrote:
> On Thu, Apr 06, 2023 at 05:20:17AM -0300, Leonardo Bras wrote:
> > While studying riscv's cmpxchg.h file, I got really interested in
> > understanding how RISCV asm implemented the different versions of
> > {cmp,}xchg.
> >
> > When I understood the pattern, it made sense for me to remove the
> > duplications and create macros to make it easier to understand what exactly
> > changes between the versions: Instruction sufixes & barriers.
> >
> > Thanks!
> > Leo
> >
> > Changes since RFCv3:
> > - Squashed the 6 original patches in 2: one for cmpxchg and one for xchg
> > https://lore.kernel.org/all/20230404163741.2762165-1-leobras@redhat.com/
> >
> > Changes since RFCv2:
> > - Fixed macros that depend on having a local variable with a magic name
> > - Previous cast to (long) is now only applied on 4-bytes cmpxchg
> > https://lore.kernel.org/all/20230321074249.2221674-1-leobras@redhat.com/
> >
> > Changes since RFCv1:
> > - Fixed patch 4/6 suffix from 'w.aqrl' to '.w.aqrl', to avoid build error
> > https://lore.kernel.org/all/20230318080059.1109286-1-leobras@redhat.com/
> >
> > Leonardo Bras (2):
> > riscv/cmpxchg: Deduplicate cmpxchg() asm and macros
> > riscv/cmpxchg: Deduplicate xchg() asm functions
> >
> > arch/riscv/include/asm/cmpxchg.h | 319 +++++++------------------------
> > 1 file changed, 67 insertions(+), 252 deletions(-)
>
> LGTM. AFAICT, this would need to be rebased, cf. e.g.
>
> a8596dda1fbf7e ("arch: rename all internal names __xchg to __arch_xchg")
>
> from the tip tree.
>
> Andrea
Thanks for the heads up!
I will update this and re-send!
And sorry about the delay :(
For some weird reason neither the cover letter, nor your message reached my
gmail, and just now looking at lore I could find your message.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v1 0/2] Deduplicating RISCV cmpxchg.h macros
2023-08-02 21:51 ` Leonardo Brás
@ 2023-08-03 3:14 ` Andrea Parri
2023-08-03 6:12 ` Leonardo Brás
0 siblings, 1 reply; 10+ messages in thread
From: Andrea Parri @ 2023-08-03 3:14 UTC (permalink / raw)
To: Leonardo Brás
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Guo Ren, Conor Dooley,
linux-riscv, linux-kernel
> > LGTM. AFAICT, this would need to be rebased, cf. e.g.
> >
> > a8596dda1fbf7e ("arch: rename all internal names __xchg to __arch_xchg")
> >
> > from the tip tree.
> >
> > Andrea
>
> Thanks for the heads up!
> I will update this and re-send!
>
>
> And sorry about the delay :(
> For some weird reason neither the cover letter, nor your message reached my
> gmail, and just now looking at lore I could find your message.
All's well that ends well. ;-) Thanks,
Andrea
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v1 0/2] Deduplicating RISCV cmpxchg.h macros
2023-08-03 3:14 ` Andrea Parri
@ 2023-08-03 6:12 ` Leonardo Brás
0 siblings, 0 replies; 10+ messages in thread
From: Leonardo Brás @ 2023-08-03 6:12 UTC (permalink / raw)
To: Andrea Parri
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Guo Ren, Conor Dooley,
Palmer Dabbelt, linux-riscv, linux-kernel
On Thu, 2023-08-03 at 05:14 +0200, Andrea Parri wrote:
> > > LGTM. AFAICT, this would need to be rebased, cf. e.g.
> > >
> > > a8596dda1fbf7e ("arch: rename all internal names __xchg to __arch_xchg")
> > >
> > > from the tip tree.
> > >
> > > Andrea
> >
> > Thanks for the heads up!
> > I will update this and re-send!
> >
> >
> > And sorry about the delay :(
> > For some weird reason neither the cover letter, nor your message reached my
> > gmail, and just now looking at lore I could find your message.
>
> All's well that ends well. ;-) Thanks,
>
> Andrea
>
Superseded by v2:
https://patchwork.kernel.org/project/linux-riscv/list/?series=772422&state=%2A&archive=both
Leo
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-08-03 6:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-06 8:20 [RFC PATCH v1 0/2] Deduplicating RISCV cmpxchg.h macros Leonardo Bras
2023-04-06 8:20 ` [RFC PATCH v1 1/2] riscv/cmpxchg: Deduplicate cmpxchg() asm and macros Leonardo Bras
2023-04-07 8:29 ` Guo Ren
2023-04-18 19:10 ` Leonardo Bras Soares Passos
2023-04-06 8:20 ` [RFC PATCH v1 2/2] riscv/cmpxchg: Deduplicate xchg() asm functions Leonardo Bras
2023-04-07 8:31 ` Guo Ren
2023-04-06 17:19 ` [RFC PATCH v1 0/2] Deduplicating RISCV cmpxchg.h macros Andrea Parri
2023-08-02 21:51 ` Leonardo Brás
2023-08-03 3:14 ` Andrea Parri
2023-08-03 6:12 ` Leonardo Brás
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).