[PATCH] D31270: [wip] IR: Move linker options to top-level global metadata and remove dllexport storage class.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added subscribers: majnemer, chandlerc.
rnk added a comment.

I'm opposed to getting rid of the dllexport bit on GlobalValue, and so far on the bug, both @majnemer and @rjmccall seem to be opposed to it as well. I also asked @chandlerc about it in person, and he wasn't in favor, although I'm not sure how strong his opinion was.

This change simplifies the job of the COFF linker because it makes the IR more like a COFF object, so I see why it's appealing. I want to rehash what I was saying about LLVM being more than just an object file format, though. LLVM is supposed to be a library. This stands in opposition to the design philosophy of LLD ELF & COFF. LLVM IR is supposed to be an intermediate representation useful for a wide variety of use cases: optimization, symbolic execution, instrumentation, and things we haven't thought of. Maintaining exports in a separate linker flag list makes it hard for these tools to determine which things are exported. Obviously, such tools can never know the true list of exports until link time, but I argue that it is useful for tools to know if *some* things are exported and have workarounds for the rest. You could argue that this is inviting bugs and such a tool needs link time information to be robust, but I'd argue that in practice it's a lot easier to do things at compile time than link time.

As a minor concern, the new representation is less efficient. We would now store mangled symbol names in one more place than we do now before, which might be significant. It seems nice to delay construction of the .drective section until after memory used by ISel has been freed.

Lastly, this is a minor bug in /msvclto, a feature that is only intended to exist until LLD can write PDBs. I feel like it just doesn't justify a representation change. Yes, LLVM IR is not cast in stone and of course should be changed to fix bugs and add features, but there is still a small cost to changing it.

---

I am in favor of moving "Linker Options" to "llvm.linker.options" if you want to do that separately.


https://reviews.llvm.org/D31270