[PATCH] D40849: CodeGen: support an extension to pass linker options on ELF

[Saleem Abdulrasool via llvm-commits]

On Fri, Dec 15, 2017 at 11:09 AM, Rui Ueyama <ruiu at google.com> wrote:

> On Fri, Dec 15, 2017 at 11:07 AM, Saleem Abdulrasool via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> compnerd added inline comments.
>>
>>
>> ================
>> Comment at: test/Feature/elf-linker-options.ll:6
>> +
>> +!0 = !{!"spaced", !"option"}
>> +!1 = !{!"nospace"}
>> ----------------
>> compnerd wrote:
>> > jhenderson wrote:
>> > > ruiu wrote:
>> > > > Imagine that these two strings are pathnames.  Pathnames can
>> contain any character other than \0. That mean you can't distinguish
>> "spaced options" from "spaced" and "options, which is bad. You should allow
>> only one string.
>> > > The way I see it, this is intended to be interpreted as a single
>> option. However, I think this demonstrates why a key-value pair is
>> important.
>> > Yes, this is a single value.  " spaced option".  I don't remember what
>> the issue was with trying to merge the entire spaced option into a single
>> string.  But, that would simplify the logic here.  I think that we should
>> go ahead and expect that the frontend will generate a single entry for
>> that.  The key/value wouldn't help with this if the spaces were removed in
>> the MDNode itself.
>> Wait, I forgot that I modelled this after the existing MachO and COFF
>> behaviors.  I think that changing the behavior of this documented interface
>> is less desirable.  If we want to have a new way to pass along the options,
>> that is fine, but should be a separate change that unifies the behavior
>> across all three object file formats.
>
>
> I don't think we should mimic an interface that could produce ambiguous
> string. If you think these interfaces need to be coherent all the time,
> please fix MachO and COFF models first
>

I understand that you are concerned about the parsing of the options in the
linker.  I think that the issue that you are raising is better addressed in
the frontend rather than in the backend.  This is purely the passthru from
the frontend to the backend, and I don't see why the LLVM IR needs to be
modified for that as it can support either approach.


>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D40849
>>
>>
>>
>>
-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171215/3f2765ff/attachment.html>