On Aug 31, 2010, at 3:30 PM, [email protected] wrote:
>
> This patch adds support for Borland extensions under the option
> -fborland-extensions, as well as semantic support for the "__pascal" calling
> convention (not yet added to llvm).
Typically, we prefer separate patches for separate changes. In this case,
-fborland-extensions in one patch and __pascal as a follow-on.
> At this time, it's not possible to disctinguish between GNU attributes
> and those of and other vendors, so the calling convention is supported
> the same as MS calling conventions like "__thiscall".
> "__attribute__((pascal))" is also supported with this patch for
> consistency - I could remove this support, but since clang seems
> to want to print "__attribute__((pascal))" for "__pascal", it made more
> sense to just allow it.
I agree that it makes sense to allow it. It's really the _pascal that we want
to enable with -fborland-extensions.
> Note from the test case that there's an issue with pointer to member
> functions which I have yet to diagnose.
I see that the same problem occurs with other calling conventions, because
we're not properly handling calling-convention attributes for member function
pointers. That's now fixed in r112715, thanks!
The patch is looking really good. There are a few small things I'd like
addressed before committing it:
The patch is missing modifications to various TableGen files, e.g., Attr.td,
Options.td, and CC1Options.td. Without those, it won't build.
Index: include/clang/Basic/TokenKinds.def
===================================================================
--- include/clang/Basic/TokenKinds.def (revision 112654)
+++ include/clang/Basic/TokenKinds.def (working copy)
@@ -335,6 +335,9 @@
KEYWORD(__thiscall , KEYALL)
KEYWORD(__forceinline , KEYALL)
+// Borland Extension.
+KEYWORD(__pascal , KEYALL)
+
// Altivec Extension.
KEYWORD(__vector , KEYALTIVEC)
KEYWORD(__pixel , KEYALTIVEC)
@@ -371,6 +374,9 @@
ALIAS("_stdcall" , __stdcall , KEYMS)
ALIAS("_thiscall" , __thiscall , KEYMS)
+// Borland Extensions.
+ALIAS("_pascal" , __pascal , KEYMS)
+
Why re-use KEYMS rather than introducing a new KEYBORLAND that is tied to
-fborland-extensions?
Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp (revision 112654)
+++ lib/CodeGen/CGCall.cpp (working copy)
@@ -36,6 +36,7 @@
case CC_X86StdCall: return llvm::CallingConv::X86_StdCall;
case CC_X86FastCall: return llvm::CallingConv::X86_FastCall;
case CC_X86ThisCall: return llvm::CallingConv::X86_ThisCall;
+ // TODO: add support for CC_X86Pascal to llvm
}
}
Typically, we'd want to emit an error here rather than silently using the wrong
calling convention. But, in this case, getting a Diagnostic object all the way
down to this routine is a pain. So, in this case, a diagnostic further up in
CodeGen, e.g., when we see a DeclRefExpr that refers to a function with
PascalAttr, would suffice.
Everything else looks great, thanks!
- Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
