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

[Rui Ueyama via llvm-commits]

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.


>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D40849
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171215/a8900f94/attachment.html>