Page MenuHomePhabricator

[Driver] Support object files in addition to static and shared libraries in compiler-rt
AcceptedPublic

Authored by phosek on Dec 21 2018, 7:21 PM.

Details

Reviewers
echristo
joerg
Summary

This change introduces support for object files in addition to static
and shared libraries which were already supported which requires
changing the type of the argument from boolean to an enum.

Diff Detail

Repository
rC Clang

Event Timeline

phosek created this revision.Dec 21 2018, 7:21 PM

Is something passing compiler-rt in as a -r link output or something?

What's the rationale for this?

This is needed for crtbegin.o/crtend.o, I originally implemented it as part of D28791 but that change is already pretty big so this is an attempt to split independent parts that could land separately into smaller changes.

Sorry, but this rather belongs to libc(abi) of Fuchsia. Why to push it into LLVM?

I don't know the original rationale of GNU.. but the final result (probably not out of interest of FSF) is that it's a vendor lock of glibc&gcc.

Please implement your crt* in your libc.

Adding joerg who raised concerns in the original change and is bypassed with this change.

I'd suggest moving this discussion to D28791 since your comments seem to be more related to the content of that change, not the content of this patch. I'm not trying to bypass the discussion in D28791, I'm just trying to split a rather large patch into smaller more easily reviewable chunks.

This patch attempts to add support for .o/object files from compiler-rt and right now there are no users and the only potential ones are crt* according to my guess.

I'd be happy to abandon this change if we decide to not move forward with D28791. I've made that change a parent revision of this change (although they don't necessarily have to land in that order) to make it obvious.

echristo accepted this revision.Mon, Dec 31, 5:27 PM

OK, that makes sense. I'm not a huge fan of how this set of code looks, but it also seems unfair to require you to extensively refactor it for this.

Any issues with crtbegin/end and where people believe things should be implemented for any given OS is orthogonal to this.

Thanks!

-eric

This revision is now accepted and ready to land.Mon, Dec 31, 5:27 PM
mstorsjo added inline comments.
clang/lib/Driver/ToolChain.cpp
376

For mingw (Triple.isWindowsGNUEnvironment) the natural extension is .o as well. So maybe check IsITANMSVCWindows instead here?

383

This looks like it's incorrectly rebased across rL343537; the non-mingw case should be .lib, not .dll - that was corrected in that commit.