This is an archive of the discontinued LLVM Phabricator instance.

Support for MSVS default calling convention options (/Gd, /Gz, /Gv, /Gr)
ClosedPublic

Authored by a.makarov on May 11 2016, 8:40 AM.

Diff Detail

Event Timeline

a.makarov retitled this revision from to Support for MSVS default calling convention options (/Gd, /Gz, /Gv, /Gr).
a.makarov updated this object.
a.makarov added a reviewer: rnk.
a.makarov added a subscriber: cfe-commits.
rnk added inline comments.May 11 2016, 8:48 AM
include/clang/Basic/LangOptions.def
219–220

Let's get rid of this and have it use a single DefaultCallingConv LangOpt.

include/clang/Driver/CC1Options.td
613

Let's unify the -cc1 layer with -mrtd, and just have a single -fdefault-calling-convention=stdcall/etc flag.

lib/AST/ASTContext.cpp
8627

I don't think we need to guard this on MSVCCompat, it'll be unset usually. It will also help us avoid a special path for MRTD.

8633–8636

Neither fastcall nor stdcall can be applied to variadic functions.

majnemer added inline comments.
lib/AST/ASTContext.cpp
8604–8606

Do these flags not have an effect on methods?

rnk added inline comments.May 11 2016, 8:56 AM
lib/AST/ASTContext.cpp
8604–8606

Nope:
https://msdn.microsoft.com/en-us/library/46t77ak2.aspx
"for all functions except C++ member functions..."

majnemer added inline comments.May 11 2016, 8:58 AM
lib/AST/ASTContext.cpp
8604–8606

Yeah, that's what the documentation says. But do they really have no effect? :)

a.makarov added inline comments.May 12 2016, 5:53 AM
lib/AST/ASTContext.cpp
8604–8606

Yes, I've checked. Member functions are not affected =)

a.makarov added inline comments.May 12 2016, 7:42 AM
lib/AST/ASTContext.cpp
8633–8636

As I've got from MSDN, vectorcall and stdcall cannot be applied to vararg functions. Are you sure about fastcall?)

Thanks for the review! I've updated the patch, please take a look.
Modifications:

  • the dependency from MS compatibility mode is removed;
  • the option is renamed into '-fdefault-calling-conv' (since it's not in MS compatibility mode);
  • the '-mrtd' option is now an alias for '-fdefault-calling-conv=stdcall';
  • 'stdcall' is now ignored for variadic functions;
  • since 'vectorcall' requires SSE2 or higher (https://msdn.microsoft.com/en-us/library/dn375768.aspx) the necessary check is added
majnemer added inline comments.May 12 2016, 10:20 AM
include/clang/Basic/LangOptions.def
175

I'd remove the "MS" from this langopt now that it helps implement -mrtd.

Renamed option 'DefaultMSCallingConv' into 'DefaultCallingConv' and enum 'DefaultMSCallingConvention' into 'DefaultCallingConvention'.

rnk added inline comments.May 13 2016, 5:17 PM
include/clang/Driver/CC1Options.td
615

Doesn't this have to live in Options.td if you want it to be available in the driver interface? Otherwise it's a -cc1-only flag.

lib/Sema/SemaDeclAttr.cpp
3898–3903

Use leading upper case variables to achieve consistency with our insanity, please. :)

I've updated the patch. Please take a look.

rnk accepted this revision.May 17 2016, 11:16 AM
rnk edited edge metadata.

Thanks, lgtm!

This revision is now accepted and ready to land.May 17 2016, 11:16 AM
This revision was automatically updated to reflect the committed changes.