This is an archive of the discontinued LLVM Phabricator instance.

[Driver] Enable selecting multiple multilibs
ClosedPublic

Authored by michaelplatings on Feb 1 2023, 3:29 AM.

Details

Summary

This will enable layering multilibs on top of each other.
For example a multilib containing only a no-exceptions libc++ could be
layered on top of a multilib containing C libs. This avoids the need
to duplicate the C library for every libc++ variant.

This change doesn't expose the functionality externally, it only opens
the functionality up to be potentially used by ToolChain classes.

Diff Detail

Unit TestsFailed

Event Timeline

michaelplatings created this revision.Feb 1 2023, 3:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2023, 3:29 AM
michaelplatings requested review of this revision.Feb 1 2023, 3:29 AM

Rebase and change llvm::SmallVector<Multilib, 1> to simply llvm::SmallVector<Multilib>

A couple of small suggestions but otherwise looks good to me.

clang/lib/Driver/ToolChains/BareMetal.cpp
228

Nit: the braces can be omitted here. No strong opinion as it may be better to keep consistency with the rest of the file rather than LLVM as a whole.

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

clang/lib/Driver/ToolChains/Fuchsia.cpp
335

SelectedMultilibs.end() - 1 makes me a little nervous. This will work for the current container type (I think the standard requires it for vector and I don't think SmallVector would deviate from it. However in the unlikely event that the container changes and this isn't valid then this could break.

Perhaps use std::advance(SelectedMultilibs.end(), -1) which is more likely to break at compile time if this occurs.

Alternatively if this is just erasing all but the last element, maybe extract it, clear the container and reinsert.

michaelplatings marked 2 inline comments as done.

Rebase and apply changes requested by @peter.smith

michaelplatings retitled this revision from [NFC] Enable selecting multiple multilibs to [Driver] Enable selecting multiple multilibs.Feb 22 2023, 8:15 AM

flags -> tags

peter.smith accepted this revision.Mar 13 2023, 9:26 AM

I've set approved from the Arm side. Please leave some time for people in the US time zone to leave any final comments or ask for extensions.

This revision is now accepted and ready to land.Mar 13 2023, 9:26 AM
phosek accepted this revision.Jun 8 2023, 12:40 AM

LGTM

This revision was landed with ongoing or failed builds.Jun 13 2023, 10:47 PM
This revision was automatically updated to reflect the committed changes.