<div dir="ltr"><div>diff --git a/include/clang/AST/Mangle.h b/include/clang/AST/Mangle.h</div><div>index 76d099f..d3a49f1 100644</div><div>--- a/include/clang/AST/Mangle.h</div><div>+++ b/include/clang/AST/Mangle.h</div><div>
@@ -108,10 +108,12 @@ public:</div><div> /// @name Mangler Entry Points</div><div> /// @{</div><div> </div><div>- virtual bool shouldMangleDeclName(const NamedDecl *D) = 0;</div><div>+ bool shouldMangleDeclName(const NamedDecl *D);</div>
<div>+ virtual bool shouldMangleDeclNameImpl(const NamedDecl *D) = 0;</div><div><br></div><div>shouldMangleDeclNameImpl can be shouldMangleCXXName, following the same logic as mangleName.</div><div> </div><div> // FIXME: consider replacing raw_ostream & with something like SmallString &.</div>
<div>- virtual void mangleName(const NamedDecl *D, raw_ostream &) = 0;</div><div>+ void mangleName(const NamedDecl *D, raw_ostream &);</div><div>+ virtual void mangleCXXName(const NamedDecl *D, raw_ostream &) = 0;</div>
<div><br></div><div><div>+enum StdOrFastCC {</div><div>+ SOF_OTHER,</div><div>+ SOF_FAST,</div><div>+ SOF_STD</div><div>+};</div><div>+</div><div>+static StdOrFastCC isStdOrFastCallFunc(const ASTContext &Context,</div>
<div>+ const NamedDecl *ND) {</div><div>+ llvm::Triple Triple = Context.getTargetInfo().getTriple();</div><div>+ if (!Triple.isOSWindows() || Triple.getArch() != llvm::Triple::x86 ||</div>
<div>+ Context.getLangOpts().CPlusPlus)</div><div>+ return SOF_OTHER;</div><div>+</div><div>+ const FunctionDecl *FD = dyn_cast<FunctionDecl>(ND);</div><div>+ if (!FD)</div><div>+ return SOF_OTHER;</div>
<div>+ QualType T = FD->getType();</div><div>+</div><div>+ if (const AttributedType *AT = dyn_cast<AttributedType>(T)) {</div><div>+ AttributedType::Kind Kind = AT->getAttrKind();</div><div>+ switch (Kind) {</div>
<div>+ default:</div><div>+ return SOF_OTHER;</div><div>+ case AttributedType::attr_stdcall:</div><div>+ return SOF_STD;</div><div>+ case AttributedType::attr_fastcall:</div><div>+ return SOF_FAST;</div>
<div>+ }</div><div>+ }</div><div><br></div><div>This AttributedType handling isn't necessary, use T->castAs<FunctionType>() instead of cast<>() to look through the type sugar.</div><div><br></div><div>
+ const FunctionType *FT = cast<FunctionType>(T);</div><div>+ CallingConv CC = FT->getCallConv();</div><div>+ switch (CC) {</div><div>+ default:</div><div>+ return SOF_OTHER;</div><div>+ case CC_X86FastCall:</div>
<div>+ return SOF_FAST;</div><div>+ case CC_X86StdCall:</div><div>+ return SOF_STD;</div><div>+ }</div><div>+}<br></div></div><div>...</div><div><div>+ const ASTContext &ASTContext = getASTContext();</div><div>
+ StdOrFastCC CC = isStdOrFastCallFunc(ASTContext, D);</div><div>+ if (CC != SOF_OTHER) {</div><div>+ ...</div><div>+ }<br></div><div>+</div><div>+ return mangleCXXName(D, Out);</div></div><div><br></div><div>We shouldn't apply the stdcall or fastcall manglings to C++ symbols. You should check if we should mangle this as a C++ name first, and after that see if we need to apply C manglings.</div>
<div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Oct 15, 2013 at 7:42 AM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">A new patch is attached. I think I implemented all of Reid's suggestions.<br>
<div class="HOEnZb"><div class="h5"><br>
On 9 October 2013 16:01, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:<br>
> This sounds like the right direction to me, if Anton's OK with removing the<br>
> mangling from LLVM.<br>
><br>
> - virtual bool shouldMangleDeclName(const NamedDecl *D) = 0;<br>
> + bool shouldMangleDeclName(const NamedDecl *D);<br>
> + virtual bool shouldMangleDeclNameImpl(const NamedDecl *D) = 0;<br>
><br>
> // FIXME: consider replacing raw_ostream & with something like<br>
> SmallString &.<br>
> - virtual void mangleName(const NamedDecl *D, raw_ostream &) = 0;<br>
> + void mangleName(const NamedDecl *D, raw_ostream &);<br>
> + virtual void mangleNameImpl(const NamedDecl *D, raw_ostream &) = 0;<br>
><br>
> Rather than mangleNameImpl, how about mangleCXXName? Basically, the shared<br>
> code is C / asm mangling only, and the ABI-specific code is all C++.<br>
><br>
> ....<br>
><br>
> +static bool isStdOrFastCallFunc(const ASTContext &Context,<br>
> + const NamedDecl *ND) {<br>
> + llvm::Triple Triple = Context.getTargetInfo().getTriple();<br>
> + if (!Triple.isOSWindows() || Triple.getArch() != llvm::Triple::x86 ||<br>
> + Context.getLangOpts().CPlusPlus)<br>
> + return false;<br>
> +<br>
> + const FunctionDecl *FD = dyn_cast<FunctionDecl>(ND);<br>
> + if (!FD)<br>
> + return false;<br>
> + QualType T = FD->getType();<br>
> + const AttributedType *AT = dyn_cast<AttributedType>(T);<br>
> + if (!AT)<br>
> + return false;<br>
> + AttributedType::Kind Kind = AT->getAttrKind();<br>
> + return Kind == AttributedType::attr_stdcall ||<br>
> + Kind == AttributedType::attr_fastcall;<br>
> +}<br>
><br>
> Don't look for the attributed type, it won't be there with -mrtd, for<br>
> example. That'd be a good test, btw. You can pull the true CC directly<br>
> from FunctionType::getCallConv().<br>
><br>
> ...<br>
><br>
> + unsigned ArgWords = 0;<br>
> + for (FunctionProtoType::arg_type_iterator Arg =<br>
> Proto->arg_type_begin(),<br>
> + ArgEnd =<br>
> Proto->arg_type_end();<br>
> + Arg != ArgEnd; ++Arg) {<br>
> + QualType AT = *Arg;<br>
> + // Size should be aligned to DWORD boundary<br>
> + ArgWords += (ASTContext.getTypeSize(AT) + 31) / 32;<br>
><br>
> Grumble grumble RoundUpToAlignment().<br>
><br>
> + }<br>
> + Out << 4 * ArgWords;<br>
><br>
> ...<br>
><br>
> diff --git a/test/CodeGen/mangle-ms.c b/test/CodeGen/mangle-ms.c<br>
> new file mode 100644<br>
> index 0000000..6706492<br>
> --- /dev/null<br>
> +++ b/test/CodeGen/mangle-ms.c<br>
><br>
> mangle-windows.c seems more accurate.<br>
><br>
><br>
><br>
> On Wed, Oct 9, 2013 at 12:45 PM, Rafael Espíndola<br>
> <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br>
>><br>
>> On 9 October 2013 14:48, Anton Korobeynikov <<a href="mailto:anton@korobeynikov.info">anton@korobeynikov.info</a>><br>
>> wrote:<br>
>> >> Doesn't mingw need these manglings to match the Windows C ABI?<br>
>> > Correct, it does need it.<br>
>><br>
>> Good catch. The attached patch handles mingw too.<br>
>><br>
>> > Another problem, which is much more severe, is that moving mangling in<br>
>> > clang will force non-clang users to handle the mangling by themselves.<br>
>><br>
>> Judging from<br>
>> <a href="http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-February/059626.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-February/059626.html</a><br>
>><br>
>> that would be an welcome change. The mangling consists of adding how<br>
>> many bytes are used for arguments. If a frontend can produce a call at<br>
>> all it has that information.<br>
>><br>
>> In any case. This patch changes only clang. Even if we decide to not<br>
>> change llvm, I think this patch is an improvement as it makes it<br>
>> possible for us to test that clang produces the desired name for C<br>
>> code (as we do for C++ already).<br>
>><br>
>> > Also, keep in mind, that gcc on linux does support stdcall / fastcall<br>
>> > calling conventions, but does not mangle the stuff.<br>
>><br>
>> That is also handled correctly.<br>
>><br>
>> Cheers,<br>
>> Rafael<br>
><br>
><br>
</div></div></blockquote></div><br></div>