Re: [PATCH] x86/altcall: use an union as register type for function parameters

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.02.2024 11:50, Roger Pau Monné wrote:
> On Thu, Feb 22, 2024 at 11:32:14AM +0100, Jan Beulich wrote:
>> On 21.02.2024 18:03, Roger Pau Monne wrote:
>>> The above can be worked around by using an union when defining the register
>>> variables, so that `di` becomes:
>>>
>>> register union {
>>>     uint8_t e;
>>>     unsigned long r;
>>> } di asm("rdi") = { .e = b };
>>>
>>> Which results in following code generated for `foo()`:
>>>
>>> foo:                                    # @foo
>>>         movzbl  %dil, %edi
>>>         callq   func
>>>         retq
>>>
>>> So the truncation is not longer lost.
>>
>> But how do you explain this behavior? I see absolutely no reason why filling
>> the one union field should lead to zero-extension. If I'm not mistaken the
>> language allows the rest of the union to retain undefined contents. So to me
>> this looks like you're converting something that failed to build due to a
>> (presumed) bug in Clang to something that any compiler would be permitted to
>> translate to other than what we want.
> 
> Oh, right, I was expecting the compiler to zero extend it, confabulating
> how unmentioned fields are initialized in structs.
> 
> However, if as mentioned in the psABI thread, the callee is required
> to do any zero extension as necessary, using the union shouldn't cause
> issues for compilers that implement the ABI properly.
> 
> IOW: I don't think the proposed workaround would cause issues for gcc.

Well, that's making a lot of assumptions then. There may indeed be little
reason for the compiler to emit different code, but it is absolutely free
to do so. I continue to think that we want to limit this workaround to as
narrow a set of compilers as possible. Clearly for Clang the code is
better than without the workaround, so there's no reason to take this as
an improvement even if in some obscure case it would still cause an issue.
But for a compiler that produces correct code without this workaround, we
shouldn't chance the workaround breaking some case somewhere.

Jan