[llvm-commits] [cfe-commits] [PATCH] Supporting thiscall compatibility with MSVC

[Eli Friedman]

On Wed, Feb 15, 2012 at 6:13 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> This is a patch for bug 10645, which adds ABI support for MSVC's
> thiscall calling convention.  Though Microsoft doesn't document this,
> the observed behavior is that *all* byval struct returns for thiscall
> methods are passed through the hidden pointer in EAX, and
> callee-cleaned.  This happens even for small structures which would
> normally be returned by EAX or EAX/EDX.
>
> There are two parts to this -- one in llvm's x86 calling convention
> tablegen (so that the hidden parameter is not passed in ecx), and the
> other is in clang (so that all byval structure return types for
> thiscall are returned by hidden pointer when in MS mode).
>
> I've got a few questions though:
>
> Should I have the tablegen only change for MS compatibility?  If so,
> I'm not certain of how to do that from the td file.

No, shouldn't be necessary...

> I'm not certain what the testcase should look like and where it should
> live (llvm or clang or both?), or is none needed for this?

Both, please.  Have a testcase that clang generates the expected IR in
clang's test/CodeGen/, and a testcase that the IR leads to the correct
x86 assembly in LLVM's test/CodeGen/X86/.

Index: lib/Target/X86/X86CallingConv.td
===================================================================
--- lib/Target/X86/X86CallingConv.td	(revision 150629)
+++ lib/Target/X86/X86CallingConv.td	(working copy)
@@ -333,6 +333,9 @@

   // The 'nest' parameter, if any, is passed in EAX.
   CCIfNest<CCAssignToReg<[EAX]>>,
+
+  // Do not pass the sret argument in ECX
+  CCIfSRet<CCDelegateTo<CC_X86_32_Common>>,

   // The first integer argument is passed in ECX
   CCIfType<[i32], CCAssignToReg<[ECX]>>,

You say "*all* byval struct returns for thiscall methods are passed
through the hidden pointer in EAX", but your patch puts the sret
argument on the stack, if I'm not mistaken...

The indentation is a bit strange in the clang patch

-Eli