On 26.07.2024 09:52, Roger Pau Monné wrote: > On Fri, Jul 26, 2024 at 09:36:15AM +0200, Jan Beulich wrote: >> On 26.07.2024 09:31, Roger Pau Monné wrote: >>> On Thu, Jul 25, 2024 at 05:00:22PM +0200, Jan Beulich wrote: >>>> On 25.07.2024 16:54, Roger Pau Monné wrote: >>>>> On Thu, Jul 25, 2024 at 03:18:29PM +0200, Jan Beulich wrote: >>>>>> On 25.07.2024 12:56, Roger Pau Monne wrote: >>>>>>> --- a/xen/arch/x86/include/asm/alternative.h >>>>>>> +++ b/xen/arch/x86/include/asm/alternative.h >>>>>>> @@ -184,11 +184,11 @@ extern void alternative_branches(void); >>>>>>> * https://github.com/llvm/llvm-project/issues/82598 >>>>>>> */ >>>>>>> #define ALT_CALL_ARG(arg, n) >>>>>>> \ >>>>>>> - register union { >>>>>>> \ >>>>>>> - typeof(arg) e[sizeof(long) / sizeof(arg)]; >>>>>>> \ >>>>>>> - unsigned long r; >>>>>>> \ >>>>>>> + register struct { >>>>>>> \ >>>>>>> + typeof(arg) e; >>>>>>> \ >>>>>>> + char pad[sizeof(void *) - sizeof(arg)]; >>>>>>> \ >>>>>> >>>>>> One thing that occurred to me only after our discussion, and I then >>>>>> forgot >>>>>> to mention this before you would send a patch: What if sizeof(void *) == >>>>>> sizeof(arg)? Zero-sized arrays are explicitly something we're trying to >>>>>> get rid of. >>>>> >>>>> I wondered about this, but I though it was only [] that we were trying >>>>> to get rid of, not [0]. >>>> >>>> Sadly (here) it's actually the other way around, aiui. >>> >>> The only other option I have in mind is using an oversized array on >>> the union, 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 >> >> Yet that's likely awful code-gen wise? > > Seems OK: https://godbolt.org/z/nsdo5Gs8W
In which case why not go this route. If the compiler is doing fine with that, maybe the array dimension expression could be further simplified, accepting yet more over-sizing? Like "sizeof(void *) / sizeof (arg) + 1" or even simply "sizeof(void *)"? Suitably commented of course ... >> For the time being, can we perhaps >> just tighten the BUILD_BUG_ON(), as iirc Alejandro had suggested? > > My main concern with tightening the BUILD_BUG_ON() is that then I > would also like to do so for the GCC one, so that build fails > uniformly. If we were to take that route, then yes, probably should constrain both (with a suitable comment on the gcc one). Jan
