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

[Eli Friedman]

On Thu, Feb 16, 2012 at 6:25 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> Here's the next attempt at the MSVC thiscall support patch, this time
> with test cases and an improved tablegen declaration.
>
> This is the first time I've done a test case for clang or llvm
> codegen, so please pay special attention to the test cases and let me
> know if I'm off-base (and what I should do differently).

You might want to make the LLVM testcase a bit stronger by using CHECK-NEXT.


+  if (callingConvention == llvm::CallingConv::X86_ThisCall &&
+      Context.getLangOptions().MicrosoftMode && RT->isStructureType()) {
+    return false;
+  }

You shouldn't make the behavior of thiscall depend on MicrosoftMode;
checking that the calling convention is thiscall should be sufficient.
 (If someone explicitly requests thiscall outside of MicrosoftMode,
they expect a Microsoft-compatible calling convention.)

The indentation in the clang patch still seems a bit weird in a few
places, like the following:

-    FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
+    FI.getReturnInfo() = classifyReturnType(FI.getReturnType(),
+                            FI.getCallingConvention());

Otherwise, this is looking good.

-Eli