[lld] r190585 - Do not hard code the leading underscore.

Rui Ueyama ruiu at google.com
Thu Sep 12 13:34:52 PDT 2013


On Thu, Sep 12, 2013 at 1:05 PM, Shankar Easwaran
<shankare at codeaurora.org>wrote:

>  Hi Rui,
>
> I think the PECOFFLinkingContext has to be derived like
> PECOFFX86_64LinkingContext and PECOFFI386_LinkingContext ?
>

It's too early to answer to that question. At this moment it seems that
PECOFFLinkingContext for x86, x86-64 and arm are not that different, so it
may not worth to make them separate subclasses. If it turns out that they
vary much, we may want to simplify the thing with subclasses. Adding a new
class hierarchy does increase complexity, so I wouldn't do it now to avoid
premature over engineering.

This would be cleaner I think.
>
> Thanks
>
> Shankar Easwaran
>
>
> On 9/12/2013 2:57 PM, Rui Ueyama wrote:
>
> On Thu, Sep 12, 2013 at 12:23 PM, Rui Ueyama <ruiu at google.com> wrote:
>
>>  Hi Joey,
>>
>>  On Thu, Sep 12, 2013 at 3:09 AM, Joey Gouly <joey.gouly at arm.com> wrote:
>>
>>> Hi Rui,
>>>
>>> > +  /// Returns the decorated name of the given symbol name. On 32-bit
>>> x86,
>>> it
>>> > +  /// adds "_" at the beginning of the string. On other architectures,
>>> the
>>> > +  /// return value is the same as the argument.
>>> > +  StringRef decorateSymbol(StringRef name) const {
>>> > +    // Because we don't support architectures other than 32-bit x86,
>>> we'll
>>> > +    // prepend an underscore unconditionally.
>>> > +    std::string str = "_";
>>> > +    str.append(name);
>>> > +    return allocateString(str);
>>> > +  }
>>> > +
>>>
>>>  The doxygen comment and the implementation don't match here. I know you
>>> said
>>> we only support
>>> 32-bit x86 for now, but how hard would it be to implement it correctly
>>> now?
>>> Or change the doxygen comment to match the current implementation?
>>>
>>
>>   The problem is that we don't have a variable indicating which platform
>> we are linking to, so we can't say wether we are producing x86-32 binary or
>> not. We assume we are always creating x86-32 binary instead in many
>> places... That being said, it should be easy to add such variable and use
>> it here. Let me do that.
>>
>
>  Done in r190628.
>
>
> _______________________________________________
> llvm-commits mailing listllvm-commits at cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130912/3f3529b1/attachment.html>


More information about the llvm-commits mailing list