Re: [PATCH for-4.19] x86/altcall: fix clang code-gen when using altcall in loop constructs

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jul 24, 2024 at 11:16:52AM +0200, Jan Beulich wrote:
> On 24.07.2024 11:13, Roger Pau Monné wrote:
> > On Wed, Jul 24, 2024 at 10:41:43AM +0200, Jan Beulich wrote:
> >> On 24.07.2024 10:28, Roger Pau Monné wrote:
> >>> The only way I've found to cope with this is to use something like:
> >>>
> >>> #define ALT_CALL_ARG(arg, n)                                            \
> >>>     union {                                                             \
> >>>         typeof(arg) e[(sizeof(long) + sizeof(arg) - 1) / sizeof(arg)];  \
> >>>         unsigned long r;                                                \
> >>>     } a ## n ## __  = {                                                 \
> >>>         .e[0] = ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); })\
> >>>     };                                                                  \
> >>>     register unsigned long a ## n ## _ asm ( ALT_CALL_arg ## n ) =      \
> >>>         a ## n ## __.r
> >>>
> >>> An oversized array that ensures all the space of the long is covered
> >>> by the array, but then we need an extra variable, as we would
> >>> otherwise require modifying ALT_CALL{0..6}_OUT to use aX_.r instead of
> >>> aX_.
> >>
> >> I don't think we need to over-size the array. It just gets yet a little
> >> more complex then:
> >>
> >> #define ALT_CALL_ARG(arg, n)                                            \
> >>     register union {                                                    \
> >>         struct {                                                        \
> >>             typeof(arg) e;                                              \
> >>             char pad[sizeof(long) - sizeof(arg)];                       \
> >>         } s;                                                            \
> >>         unsigned long r;                                                \
> >>     } a ## n ## _ asm ( ALT_CALL_arg ## n ) = {                         \
> >>         .s.e = ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); }) \
> >>     }
> > 
> > We could even simplify this, there's no need for the union anymore,
> > since struct size == sizeof(long) already:
> > 
> > #define ALT_CALL_ARG(arg, n)                                            \
> >     register struct {                                                   \
> >         typeof(arg) e;                                                  \
> >         char pad[sizeof(long) - sizeof(arg)];                           \
> >     } a ## n ## _ asm ( ALT_CALL_arg ## n ) = {                         \
> >         .e = ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); })   \
> >     }
> > 
> > The above seems to work for both the original issue and the new one.
> 
> Oh, good. If the compiler's happy with not being dealt an unsigned long,
> even better. Maybe for consistency we want to use sizeof(void *) also in
> pad's definition then.

I have to test this on Gitlab, it seems to be fine with clang version
18.1.6, not sure about older releases.

I can switch to void *, that would be fine, will attempt to prepare a
followup patch.

Thanks, Roger.