[cfe-commits] [PATCH][CodeGen] Add padding to expanded struct function arguments if the ABI requires it.

[John McCall]

On Nov 13, 2010, at 9:43 AM, Michael Spencer wrote:

> On Sat, Nov 13, 2010 at 1:12 AM, John McCall <rjmccall at apple.com> wrote:
>> 
>> On Nov 12, 2010, at 7:52 PM, Michael Spencer wrote:
>> 
>>> On Fri, Nov 12, 2010 at 10:14 PM, John McCall <rjmccall at apple.com> wrote:
>>>> On Oct 18, 2010, at 11:54 PM, Michael Spencer wrote:
>>>>> The Microsoft ABI requires that structs passed by value as "expanded"
>>>>> arguments maintain padding. This patch implements this and fixes
>>>>> <http://llvm.org/bugs/show_bug.cgi?id=8398>.
>>>> 
>>>> Sorry this has taken so long.  Can you explain how this approach is different from just not passing the struct as expanded at all, i.e. passing it byval?
>>>> 
>>>> John.
>>> 
>>> I thought byval was a pointer thing, would it work in this case?
>> 
>> As I understand it, byval arguments are passed in memory by copying the pointed-to object into the appropriate position in the arguments, which is pretty much exactly what you want.  I think the pointer-is-secretly-a-struct thing is just a throwback to before we had first-class aggregates.
>> 
>> I'm also pretty sure that expansion won't work because it screws up register-passing CCs, of which there are several we need to support on MS platforms.  For example:
>> 
>>  struct A { int x; double d; };
>>  void __fastcall foo(struct A, void *, void *);  // the pointers should be passed in registers, not the int and the padding pseudo-argument.
>> 
>> John.
> 
> OK, so the actual problem is in X86_32ABIInfo::classifyArgumentType.
> It should not be expanding small structs with padding, or any struct
> at all when using fastcc.

_fastcall is x86_fastcall;  fastcc is a totally different beast.

The way I read it, the Windows calling conventions *never* conceptually do struct expansion;  i.e. in no case do you ever pass a struct by breaking it into components and figuring out how to pass those individually.  (In contrast, the standard Unix x86-64 CC will pass, e.g., struct { double d; void *p; } in an FP register and an integer register unless you're passing it as a vararg).  I think that's how the standard Unix x86 convention works, too.  That doesn't mean we never want to do struct expansion anyway — it generally produces cleaner IR and allows better optimization and codegen — but it does mean we have to be careful to only do it in cases where the backend will end up producing the right results.  That probably means a lot of experimentation, but that can happen later, since it's just QoI;  for now, never expanding for structs with padding sounds good.

The documentation for __fastcall that I've seen says that arguments are passed in registers when they're no larger than a pointer;  if that's true and small structs really are passed in a single register, then classifyArgumentType should return 'direct' with a coercion to the appropriate integer type.  It's probably okay to do this regardless of CC and clean it up later as QoI work, but however you see fit.

Some useful test cases to investigate:

void _fastcall foo(struct A { char a; short b; } var);
void _fastcall foo(struct A { short a; short b; } var);

John.