Re: [PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers

[Xi Ruoyao <xry111@xry111.site>]

On Thu, 2025-06-05 at 09:37 +0200, Thomas Weißschuh wrote:
> On Wed, Jun 04, 2025 at 10:30:55PM +0800, Xi Ruoyao wrote:
> > On Wed, 2025-06-04 at 22:05 +0800, Huacai Chen wrote:
> > > On Tue, Jun 3, 2025 at 7:49 PM Thomas Weißschuh
> > > <thomas.weissschuh@linutronix.de> wrote:
> > > > 
> > > > The syscall wrappers use the "a0" register for two different register
> > > > variables, both the first argument and the return value. The "ret"
> > > > variable is used as both input and output while the argument register is
> > > > only used as input. Clang treats the conflicting input parameters as
> > > > undefined behaviour and optimizes away the argument assignment.
> > > > 
> > > > The code seems to work by chance for the most part today but that may
> > > > change in the future. Specifically clock_gettime_fallback() fails with
> > > > clockids from 16 to 23, as implemented by the upcoming auxiliary clocks.
> > > > 
> > > > Switch the "ret" register variable to a pure output, similar to the other
> > > > architectures' vDSO code. This works in both clang and GCC.
> > > Hmmm, at first the constraint is "=r", during the progress of
> > > upstream, Xuerui suggested me to use "+r" instead [1].
> > > [1]  https://lore.kernel.org/linux-arch/5b14144a-9725-41db-7179-c059c41814cf@xen0n.name/
> > 
> > Based on the example at
> > https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html:
> > 
> >    To force an operand into a register, create a local variable and specify
> >    the register name after the variable’s declaration. Then use the local
> >    variable for the asm operand and specify any constraint letter that
> >    matches the register:
> >    
> >    register int *p1 asm ("r0") = …;
> >    register int *p2 asm ("r1") = …;
> >    register int *result asm ("r0");
> >    asm ("sysint" : "=r" (result) : "0" (p1), "r" (p2));
> >    
> > I think this should actually be written
> > 
> >  	asm volatile(
> >  	"      syscall 0\n"
> > 	: "=r" (ret)
> >  	: "r" (nr), "0" (buffer), "r" (len), "r" (flags)
> >  	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
> > "$t8",
> >  	  "memory");
> > 
> > i.e. "=" should be used for the output operand 0, and "0" should be used
> > for the input operand 2 (buffer) to emphasis the same register as
> > operand 0 is used.
> 
> I would have expected that matching constraints ("0") would only really make
> sense if the compiler selects the specific register to use. When the register is
> already selected manually it seems redundant.
> But my inline ASM knowledge is limited and this is a real example from the GCC
> docs, so it is probably more correct.
> On the other hand all the other vDSO implementations use "r" over "0" for the
> input operand 2 and I'd like to keep them consistent.

Per https://gcc.gnu.org/pipermail/gcc-help/2025-June/144261.html and
https://gcc.gnu.org/pipermail/gcc-help/2025-June/144266.html it should
be fine to just use "r".

Reviewed-by: Xi Ruoyao <xry111@xry111.site>

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University