Microsoft X64 Mangling

[Warren Hunt]

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/3ab0f4ff/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: x64mangle3.patch
Type: application/octet-stream
Size: 21007 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130510/3ab0f4ff/attachment.obj>