Re: [PATCH v4 05/21] arm64: cmpxchg128: LSE: Remove redundant operands

[Mark Rutland <mark.rutland@arm.com>]

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@arm.com>

Thanks!

Mark.

> 
> > +	: [new1] "r" (x2), [new2] "r" (x3)				\
> >  	: cl);								\
> >  									\
> >  	r.low = x0; r.high = x1;					\
> > -- 2.30.2
> > 
>