[PATCH] D114536: [X86][MS] Fix the wrong alignment of vector variable arguments on Win32

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

In D114536#3191746 <https://reviews.llvm.org/D114536#3191746>, @pengfei wrote:

> Sorry, I made a mistake when I wanted to demonstrate the difference between variant and fixed arguments. Yes, you are right. The alignment I showed in `f2` is the store of variable instead of ABI's.
> A summary from my latest investigation:
>
> 1. For fixed function arguments:
>   - LLVM will pass the first 3 vector variables by register: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/X86/X86CallingConv.td#L804
>   - LLVM will pass the following vector variables by value with alignment = 4: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/X86/X86CallingConv.td#L788
>   - Clang FE will emit the address of value instead of the value itself. So we don't have chance to handle the alignment. https://godbolt.org/z/KvvY4hMda
>   - This is matching what MSVC's doing.
> 2. For variant arguments:
>   - MSVC allows 3 vector variables at max no matter whether the variables are in the left of comma or in ellipsis in the prototype: https://godbolt.org/z/sYbcheEjv
>   - MSVC always use stack to pass vector variables. The alignment for the vector variables is 4.
>
>> I think what I'm trying to get at is that, in these two prototypes, v should be passed identically:
>
> No, they are not. On fixed arguments function, vector variables are passed by registers or address. While on variant function, vector variables are limited to 3 and passed by stack.
>
>> I'm saying that, this code, which stores an argument to stack memory, should maybe clamp its alignment assumption to the ABI's stack alignment, which on Windows, happens to be 4. On most other platforms, it will be 16.
>
> We have specified each of the type's alignment in the calling conversion when they are passed by stack: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/X86/X86CallingConv.td#L841
> You can find on other 32 bit platforms, the type alignments are almost 4 too. The only exception is `f80`. Anyway, we don't need to warry about the fixed arguments here.
>
>> The IsVarArg boolean affects the entire call site.
>
> Yes, because all vector variables are passed on stack with alignment = 4. No matter they are in the left of comma or right. It is intended to affects the entire call site. .
>
>> Adding and removing an ellipsis to the prototype should not change the instructions used to store the prototyped arguments, they should remain the same, and use the regular alignment assumptions, right?
>
> No. Adding and removing an ellipsis are totally different ways when passing arguments. We need to use IsVarArg to distinguish each other.

Thanks for the info, sorry about the delayed response. I see that adding the ellipsis drastically changes the way vectors are passed, but I think that complexity lives in the frontend. If the function is vararg, the frontend (Clang or other) will pass the vector directly. If it has a fixed prototype, the vector should be passed by address after passing three vectors.

There are no cases when a vector passed on the stack is aligned to 16 bytes, it should always be four byte aligned. Therefore, I don't think we need the `IsVarArg` boolean, we can go ahead and clamp the alignment on these argument loads.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114536/new/

https://reviews.llvm.org/D114536