Microsoft X64 Mangling
Richard Smith
richard at metafoo.co.uk
Fri May 10 14:46:13 PDT 2013
LGTM
As we discussed offline, we should check whether the E is present on a
pointer-to-member-function mangling (if so, maybe it should be added when
mangling the function type, not when mangling the function decl).
On Fri, May 10, 2013 at 2:37 PM, Warren Hunt <whunt at google.com> wrote:
> Addressing BNF comments request.
>
> -Warren
>
>
> On Thu, May 9, 2013 at 3:49 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>
>> On Thu, May 9, 2013 at 11:32 AM, Warren Hunt <whunt at google.com> wrote:
>>
>>> I updated the white space.
>>>
>>> Are these are the checks for pointers/references to functions that you
>>> were looking for? (lines 128-134 of mangle-ms-arg-qualifiers.cpp)
>>>
>>
>> Ah, yes, thanks :)
>>
>>
>>> void foo_p6ahxz(int x()) {}
>>> // CHECK: "\01?foo_p6ahxz@@YAXP6AHXZ at Z"
>>> // X64: "\01?foo_p6ahxz@@YAXP6AHXZ at Z"
>>>
>>> void foo_a6ahxz(int (&x)()) {}
>>> // CHECK: "\01?foo_a6ahxz@@YAXA6AHXZ at Z"
>>> // X64: "\01?foo_a6ahxz@@YAXA6AHXZ at Z"
>>>
>>> void foo_q6ahxz(int (&&x)()) {}
>>> // CHECK: "\01?foo_q6ahxz@@YAX$$Q6AHXZ at Z"
>>> // X64: "\01?foo_q6ahxz@@YAX$$Q6AHXZ at Z"
>>>
>>>
>>>
>>> On Wed, May 8, 2013 at 3:39 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>>>
>>>> @@ -1277,6 +1284,8 @@ void
>>>> MicrosoftCXXNameMangler::mangleFunctionClass(const FunctionDecl *FD) {
>>>> else
>>>> Out << 'Q';
>>>> }
>>>> + if (PointersAre64Bit && !MD->isStatic())
>>>> + Out << 'E';
>>>> } else
>>>> Out << 'Y';
>>>> }
>>>>
>>>> Needs more indentation.
>>>>
>>>> You have special cases for pointers/references to functions (where no E
>>>> is added) but I don't see any test cases for that.
>>>>
>>>> The BNF comments should be updated in various places to indicate the
>>>> presence of this specifier. It might also be useful to factor out the "if
>>>> (PointersAre64Bit) Out << 'E';" code into a function, and make up a name
>>>> for it to use in the BNF descriptions.
>>>>
>>>
>> I'd still like these comments updated. Other than that, patch looks fine.
>>
>>
>>> On Wed, May 8, 2013 at 1:36 PM, Warren Hunt <whunt at google.com> wrote:
>>>>
>>>>> ping
>>>>>
>>>>>
>>>>> On Fri, May 3, 2013 at 12:07 PM, Warren Hunt <whunt at google.com> wrote:
>>>>>
>>>>>> Sounds good. I added a local that caches the 64-bitness of pointers.
>>>>>>
>>>>>> -Warren
>>>>>>
>>>>>>
>>>>>> On Thu, May 2, 2013 at 2:51 PM, Charles Davis <cdavis5x at gmail.com>wrote:
>>>>>>
>>>>>>>
>>>>>>> On May 2, 2013, at 3:19 PM, Warren Hunt wrote:
>>>>>>>
>>>>>>> > I've put together a patch that fixes all of the issues I found
>>>>>>> with Microsoft name mangling in 64-bit mode. Mostly it involves adding an
>>>>>>> 'E' note to all pointers that are 64-bit (including 'this' pointers). This
>>>>>>> is done in MicrosoftMangle.cpp. I've updated 3 of the mangling test files
>>>>>>> with X64 tests, which provides some reasonable amount of coverage.
>>>>>>> >
>>>>>>> > Please review.
>>>>>>> >
>>>>>>> > Thanks,
>>>>>>> > -Warren
>>>>>>> >
>>>>>>> > (Note: This is my first patch.)
>>>>>>> Welcome to Clang development!
>>>>>>>
>>>>>>> > @@ -1277,6 +1278,9 @@ void
>>>>>>> MicrosoftCXXNameMangler::mangleFunctionClass(const FunctionDecl *FD) {
>>>>>>> > else
>>>>>>> > Out << 'Q';
>>>>>>> > }
>>>>>>> > + if (getASTContext().getTargetInfo().getPointerWidth(0) == 64 &&
>>>>>>> > + !MD->isStatic())
>>>>>>> > + Out << 'E';
>>>>>>> > } else
>>>>>>> > Out << 'Y';
>>>>>>> > }
>>>>>>> The indentation on this looks wrong. (You can't tell from here, but
>>>>>>> it's definitely wrong in the patch itself.) Also, I think we should handle
>>>>>>> this where we mangle in the 'this' qualifiers--the 'E' is because the
>>>>>>> 'this' pointer is Extended (__ptr64 instead of __ptr32). (The relevant line
>>>>>>> is 1172.)
>>>>>>>
>>>>>>> > @@ -1390,6 +1394,8 @@ void
>>>>>>> MicrosoftCXXNameMangler::mangleDecayedArrayType(const ArrayType *T,
>>>>>>> > manglePointerQualifiers(T->getElementType().getQualifiers());
>>>>>>> > } else {
>>>>>>> > Out << 'Q';
>>>>>>> > + if (getASTContext().getTargetInfo().getPointerWidth(0) == 64)
>>>>>>> > + Out << 'E';
>>>>>>> > }
>>>>>>> > mangleType(T->getElementType(), SourceRange());
>>>>>>> > }
>>>>>>> Why only here? What does VC do in the global case?
>>>>>>>
>>>>>>> There's a lot of code duplication (i.e. checks against the pointer
>>>>>>> size sprinkled throughout the code). Most of those can be collapsed into
>>>>>>> mangleQualifiers().
>>>>>>>
>>>>>>> Someone else wrote a patch a while back. You can find it here:
>>>>>>>
>>>>>>> http://llvm-reviews.chandlerc.com/D101
>>>>>>>
>>>>>>> I didn't finish reviewing it because real life got in the way. :\.
>>>>>>> You might find some ideas in there.
>>>>>>>
>>>>>>> Chip
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> cfe-commits mailing list
>>>>> cfe-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130510/a59149a4/attachment.html>
More information about the cfe-commits
mailing list