Page MenuHomePhabricator

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

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



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


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.Dec 31 2018, 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.



This revision is now accepted and ready to land.Dec 31 2018, 5:27 PM
mstorsjo added inline comments.
376 ↗(On Diff #179403)

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

383 ↗(On Diff #179403)

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

phosek updated this revision to Diff 189552.Mar 6 2019, 12:13 PM
phosek marked 2 inline comments as done.
Herald added a project: Restricted Project. · View Herald TranscriptMar 6 2019, 12:13 PM
Herald added a subscriber: jdoerfert. · View Herald Transcript
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 11 2019, 7:14 PM