Re: [PATCH 12/12] [PROBABLY WRONG] s390: void '0' constraint in inline assembly

[Arnd Bergmann]

On Wed, Apr 10, 2019 at 3:55 PM Martin Schwidefsky
<schwidefsky@xxxxxxxxxx> wrote:
>
> On Mon,  8 Apr 2019 23:26:25 +0200
> Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> > diff --git a/arch/s390/include/asm/processor.h b/arch/s390/include/asm/processor.h
> > index 700c650ffd4f..84c59c99668a 100644
> > --- a/arch/s390/include/asm/processor.h
> > +++ b/arch/s390/include/asm/processor.h
> > @@ -262,7 +262,7 @@ static __no_kasan_or_inline unsigned short stap(void)
> >       register unsigned long r4 asm("6") = (unsigned long)(arg5)
> >
> >  #define CALL_FMT_0
> > -#define CALL_FMT_1 CALL_FMT_0, "0" (r2)
> > +#define CALL_FMT_1 CALL_FMT_0, "d" (r2)
> >  #define CALL_FMT_2 CALL_FMT_1, "d" (r3)
> >  #define CALL_FMT_3 CALL_FMT_2, "d" (r4)
> >  #define CALL_FMT_4 CALL_FMT_3, "d" (r5)
>
> This is (slightly) wrong. %r2 is used as the input register for the first argument
> and the result value for the call. With your patch you force the compiler to load
> the first argument in two registers. One solution would be to CALL_FMT1 as
>
>         #define CALL_FMT1 CALL_FMT_0
>
> It still is not optimal though as for CALL_FMT_0 the "+&d" (r2) indicates an
> input but CALL_ARGS_0 does not initialize r2.

Ok, thanks for taking a closer look!

> I am thinking about the following patch to cover all cases:
> --
> From 91a4abbec91a9f26f84f7386f2c0f96de669b0eb Mon Sep 17 00:00:00 2001
> From: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
> Date: Wed, 10 Apr 2019 15:48:43 +0200
> Subject: [PATCH] s390: fine-tune stack switch helper
>
> The CALL_ON_STACK helper currently does not work with clang and for
> calls without arguments it does not initialize r2 although the contraint
> is "+&d". Rework the CALL_FMT_x and the CALL_ON_STACK macros to work
> with clang and produce optimal code in all cases.
>
> Reported-by: Arnd Bergmann <arnd@xxxxxxxx>
> Signed-off-by: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>

I did another build test to confirm that your patch works fine with clang
as well, looks good to me.

      Arnd