Include calling convention in function print

[Quuxplusone]

Bugzilla Link	PR28628
Status	NEW
Importance	P enhancement
Reported by	Andrew Artz (keyboardsmoke@gmail.com)
Reported on	2016-07-20 10:37:52 -0700
Last modified on	2016-07-20 11:35:36 -0700
Version	trunk
Hardware	All All
CC	llvm-bugs@lists.llvm.org, rnk@google.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Unsure if the option in PrettyPrinter.h is necessary, but I wanted it in my own
space. If there was already a way to specify my intention to print the calling
convention, please inform me and close this - but I didn't see one.

Index: PrettyPrinter.h
===================================================================
--- PrettyPrinter.h (revision 275612)
+++ PrettyPrinter.h (working copy)
@@ -50,7 +50,7 @@
       UseVoidForZeroParams(!LO.CPlusPlus),
       TerseOutput(false), PolishForDeclaration(false),
       Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
-      IncludeNewlines(true), MSVCFormatting(false) { }
+      IncludeNewlines(true), IncludeCallingConvention(false),
MSVCFormatting(false) { }

   /// \brief Adjust this printing policy for cases where it's known that
   /// we're printing C++ code (for instance, if AST dumping reaches a
@@ -196,6 +196,9 @@
   /// \brief When true, include newlines after statements like "break", etc.
   unsigned IncludeNewlines : 1;

+  /// \brief When true, include calling convention information for functions.
+  unsigned IncludeCallingConvention : 1;
+
   /// \brief Use whitespace and punctuation like MSVC does. In particular, this
   /// prints anonymous namespaces as `anonymous namespace' and does not insert
   /// spaces after template arguments.

Index: DeclPrinter.cpp
===================================================================
--- DeclPrinter.cpp (revision 275612)
+++ DeclPrinter.cpp (working copy)
@@ -620,6 +640,68 @@
         Out << "auto " << Proto << " -> ";
         Proto.clear();
       }
+
+     //
+     if (Policy.IncludeCallingConvention)
+     {
+         CallingConv cconv = AFT->getCallConv();
+
+         std::string ccname = AFT->getNameForCallConv(cconv).str();
+
+         if (Policy.MSVCFormatting)
+         {
+             switch (cconv)
+             {
+             case CC_C:
+             case CC_X86StdCall:
+             case CC_X86FastCall:
+             case CC_X86Pascal: // Not supported anymore.
+                 ccname = "__" + ccname;
+                 break;
+             case CC_X86ThisCall:
+                 if (!D->isCXXClassMember())
+                     ccname = "__" + ccname;
+                 else
+                     ccname = ""; // __thiscall is the default (at least for MSVC...)
+
+                 break;
+             case CC_X86VectorCall:
+                 ccname = "_" + ccname;
+                 break;
+             default:
+                 // CC_X86Pascal
+                 // CC_X86_64Win64
+                 // CC_X86_64SysV
+                 // CC_AAPCS
+                 // CC_AAPCS_VFP
+                 // CC_IntelOclBicc
+                 // CC_SpirFunction
+                 // CC_OpenCLKernel
+                 // CC_Swift
+                 // CC_PreserveMost
+                 // CC_PreserveAll
+                 ccname = ""; // Not supported.
+                 break;
+             }
+         }
+         else
+         {
+             if (cconv == CC_AAPCS)
+                 ccname = "__attribute__((pcs(\"aapcs\")))";
+             else if (cconv == CC_AAPCS_VFP)
+                 ccname = "__attribute__((pcs(\"aapcs-vfp\")))";
+             else if (cconv == CC_SpirFunction)
+                 ccname = ""; // default for OpenCL functions on SPIR target
+             else if (cconv == CC_OpenCLKernel)
+                 ccname = ""; // inferred for OpenCL kernels
+             else
+                ccname = "__attribute__((" + ccname + "))";
+         }
+
+         if (!ccname.empty())
+             Proto = ccname + " " + Proto;
+     }
+
       AFT->getReturnType().print(Out, Policy, Proto);
       Proto.clear();
     }

[Quuxplusone]

I thought we already handled it in TypePrinter, but maybe DeclPrinter doesn't use that for printing declarators.

[Quuxplusone]

Just noticed I left in CC_X86Pascal for MSVC. Awkward, it shouldn't be an issue unless some calling convention is found to be __pascal by clang. Unlikely but possible.

printFunctionProtoAfter does contain some functionality to add the calling convention, with no MSVC support (it seems?). I haven't been using the TypePrinter, but perhaps it might be better to move over and attempt to rectify some of these issues there as well.

[Quuxplusone]

Well after some chatting in #llvm I am lead to believe patches like these aren't exactly necessary because MSVC compat in printing isn't something that clang is reaching for right now.

The bug itself is not invalid, I believe - because outside of TypePrinter calling conventions aren't printed which may not be desirable, but perhaps some of the MSVC specific compatibility stuff can be stripped out.

[Quuxplusone]

Let's try something more like this, then...
I would have reused TypePrinter stuff, but it isn't exposed in a manner that
makes this simple.

This fixes the 'bug' without introducing too much headache, whether or not an
option is warranted isn't up to me, I suppose, but the TypePrinter does it
without an option so it should be fine.

Index: DeclPrinter.cpp
===================================================================
--- DeclPrinter.cpp (revision 275612)
+++ DeclPrinter.cpp (working copy)
@@ -620,6 +640,24 @@
         Out << "auto " << Proto << " -> ";
         Proto.clear();
       }
+
+      CallingConv cconv = AFT->getCallConv();
+      std::string ccname = AFT->getNameForCallConv(cconv).str();
+
+      if (cconv == CC_AAPCS)
+        ccname = "__attribute__((pcs(\"aapcs\")))";
+      else if (cconv == CC_AAPCS_VFP)
+        ccname = "__attribute__((pcs(\"aapcs-vfp\")))";
+      else if (cconv == CC_SpirFunction)
+        ccname = ""; // default for OpenCL functions on SPIR target
+     else if (cconv == CC_OpenCLKernel)
+        ccname = ""; // inferred for OpenCL kernels
+      else
+        ccname = "__attribute__((" + ccname + "))";
+
+      if (!ccname.empty())
+        Proto = ccname + " " + Proto;
+
       AFT->getReturnType().print(Out, Policy, Proto);
       Proto.clear();
     }