| Message ID | 20230406082018.70367-3-leobras@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | Deduplicating RISCV cmpxchg.h macros | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD d34a6b715a23 |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 1 and now 1 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 3323 this patch: 3323 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 17794 this patch: 17794 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 170 lines checked |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
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>
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) \ ({ \
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(-)