[PATCH v4 05/21] arm64: cmpxchg128: LSE: Remove redundant operands
Mark Rutland
mark.rutland at arm.com
Fri Sep 11 04:43:15 PDT 2026
On Thu, Sep 10, 2026 at 11:06:48AM +0100, Vladimir Murzin wrote:
> On 9/8/26 16:17, Mark Rutland wrote:
> > @@ -291,15 +291,13 @@ __lse__cmpxchg128##name(volatile u128 *ptr, u128 old, u128 new) \
> > register unsigned long x1 asm ("x1") = o.high; \
> > register unsigned long x2 asm ("x2") = n.low; \
> > register unsigned long x3 asm ("x3") = n.high; \
> > - register unsigned long x4 asm ("x4") = (unsigned long)ptr; \
> > \
> > asm volatile( \
> > __LSE_PREAMBLE \
> > " casp" #mb "\t%[old1], %[old2], %[new1], %[new2], %[v]\n"\
> > : [old1] "+&r" (x0), [old2] "+&r" (x1), \
> > [v] "+Q" (*(u128 *)ptr) \
> > - : [new1] "r" (x2), [new2] "r" (x3), [ptr] "r" (x4), \
> > - [oldval1] "r" (o.low), [oldval2] "r" (o.high) \
>
> Both 'old1' and #old are read and write and that reflected by '+' constraint modifier.
> However, I do not fully understand why we need earlyclobber for them:
> - they are not written before read
> - we already constrained registers, so they do not overlap
> - cmpxchg few lines above doesn't need it
I agree that the earlyclobber shouldn't be necessary for 'old1' or
'old2'. I expect it shouldn't have a negative effect on code generation
here, as removing it doesn't give the compiler any new freedom to
allocate registers differently, but maybe it causes the compiler to do a
bit more work internally.
I'll leave this patch as-is for now, but I am happy to delete the
earlyclobber in a subsequent patch (or to ack a patch doing so), if
people want?
> I had a look at ll/sc counterpart and both earlyclobebr and '=' constraint modifier
> make sense there, since we do not want registers to overlap and they are written.
>
> I admit might be missing something. Anyway, FWIW
I don't think you're missing anything here. I suspect we just
copy-pasted the LL/SC constraints without thinking too hard when this
was added in commit:
b23e139d0b66c021 ("arch: Introduce arch_{,try_}_cmpxchg128{,_local}()")
> Reviewed-by: Vladimir Murzin <vladimir.murzin at arm.com>
Thanks!
Mark.
>
> > + : [new1] "r" (x2), [new2] "r" (x3) \
> > : cl); \
> > \
> > r.low = x0; r.high = x1; \
> > -- 2.30.2
> >
>
More information about the linux-arm-kernel
mailing list