On Thu, May 29, 2025 at 03:07:34PM -0500, Peter Bergner wrote:
> On 5/29/25 5:35 AM, Segher Boessenkool wrote:
> >
> > Add yourself to suthors as well?
>
> Agreed. Just add your name/email address directly under mine, like so:
>
> 2025-05-29 Peter Bergner <berg...@linux.ibm.com>
> Jeevitha Palanisamy <jeevi...@linux.ibm.com>
Like that.
> >> +{ \
> >> + register TYPE _ret asm ("r8"); \
> >> + register TYPE _cond asm ("r9") = _COND; \
> >> + register TYPE _value asm ("r10") = _VALUE;
> >> \
> >> + __asm__ __volatile__ (OPCODE " %[ret],%P[addr],%[code]" \
> >> + : [addr] "+Q" (_PTR[0]), [ret] "=r" (_ret) \
> >> + : "r" (_cond), "r" (_value), [code] "n" (FC)); \
> >> + return _ret;
> >> \
> >> +}
> >
> > Naming the operands is an extra indirection, and makes things way less
> > readable (which means *understandable*) as well. Just use %0, %1, %2
> > please? It's a single line, people will not lose track of what is what
> > anyway (and if they would, the code is then way too big for extended
> > asm, so named asm operands is always a code stench).
>
> I agree that's a little too much syntactic sugar, but we were just
Sugar is an addictive useless slow-killing substance. This is more like
syntactic fentanyl.
> being consistent with the other existing code that uses this syntax.
> I suppose you could use %1,%0,%4 here (%2 & %3 are not used directly)
> and then clean up the other code similarly as a follow-on cleanup?
It would be nice to get %0,%1,%4 even, like we have in all similar
patterns.
Operands in order, and %0 is the leftmost one, the destination reg.
> >> +#define _AMO_LD_INCREMENT(NAME, TYPE, OPCODE, FC) \
> >> +static __inline__ TYPE
> >> \
> >> +NAME (TYPE *_PTR) \
> >> +{ \
> >> + TYPE _RET;
> >> \
> >> + __asm__ volatile (OPCODE " %[ret],%P[addr],%[code]\n"
> >> \
> >> + : [addr] "+Q" (_PTR[0]), [ret] "=r" (_RET) \
> >> + : "Q" (*(TYPE (*)[2]) _PTR), [code] "n" (FC)); \
> >> + return _RET;
> >> \
> >> +}
> >
> > I don't understand the [2]. Should it be [1]? These instructions
> > can use the value at mem+s (as the ISA names things) as input, but not
> > mem+2*s.
>
> I think 2 is correct here. This 2 isn't an index like the 0 in _PTR[0],
> but it's a size.
Yeah, it's a declarator here (a type-name), inside a cast. Nastiness.
Oh well, hidden in a macro ;-)
> This specific use is trying to say we're reading from
> memory and we're reading 2 locations, mem(EA,s) and mem(EA+s,s).
> Maybe we could use separate mentions of _PTR[0] and _PTR[1] instead???
As memory operands? Yeah that could be cleaner.
> We don't actually use that "operand" in the instruction, it's just there
> to tell the compiler that those memory locations are read.
>
> Ditto for _AMO_LD_DECREMENT usage, which reads mem(EA-s,s) and mem(EA,s).
Yup.
Segher
