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>
>> +{ \
>> + 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
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?
>> +#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. 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???
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).
Peter
