This is an archive of the discontinued LLVM Phabricator instance.

Add default calling convention support for regcall.
ClosedPublic

Authored by eandrews on Oct 23 2017, 3:57 PM.

Details

Summary

Added support for regcall as default calling convention. Also added code to exclude main when applying default calling conventions.

Diff Detail

Repository
rL LLVM

Event Timeline

eandrews created this revision.Oct 23 2017, 3:57 PM
rnk added inline comments.Oct 23 2017, 5:09 PM
lib/Sema/SemaType.cpp
3269–3273 ↗(On Diff #119957)

I highly doubt this is correct. I have a feeling there are all kinds of ways you can get this to fire on things that aren't a function declaration. It's also inefficient to check the identifier string every time we make a function type. Please find somewhere else to add this. I'd suggest adjusting the function type in CheckMain, or some time before then, whereever we make main implicitly extern "C".

erichkeane added inline comments.Oct 23 2017, 5:15 PM
lib/Sema/SemaType.cpp
3269–3273 ↗(On Diff #119957)

I believe the logic here was pulled from FunctionDecl's "isMain" function: https://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html#aa2b31caf653741632b16cce1ae2061cc

As this is so early in the process (at Declarator), I didn't see a good place to recommend extracting this, besides pulling it into the Declarator.

CheckMain is also run at the Decl stage, isn't it? Is your suggestion be to simply let this go through with the wrong calling-convention here, then revert it it in "CheckMain"?

rnk added inline comments.Oct 23 2017, 5:29 PM
lib/Sema/SemaType.cpp
3269–3273 ↗(On Diff #119957)

Yep. You can look at how we use FunctionTypeUnwrapper and ASTContext::adjustFunctionType in various places to fix up function types that were built with the wrong calling convention without losing type source info.

erichkeane added inline comments.Oct 23 2017, 6:14 PM
lib/Sema/SemaType.cpp
3269–3273 ↗(On Diff #119957)

Perfect, thanks for the examples! I am sure @eandrews can use those to track down the right place to fix this up.

eandrews updated this revision to Diff 120284.Oct 25 2017, 10:58 AM

Updated patch to set default calling convention for main() in CheckMain()

eandrews added inline comments.Oct 25 2017, 11:00 AM
lib/Sema/SemaType.cpp
3269–3273 ↗(On Diff #119957)

Thanks for the review! I've updated the patch. Please take a look.

rnk accepted this revision.Nov 2 2017, 1:00 PM

Looks good! Sorry for the delay.

This revision is now accepted and ready to land.Nov 2 2017, 1:00 PM

No problem! Thanks!

This revision was automatically updated to reflect the committed changes.