This is an archive of the discontinued LLVM Phabricator instance.

BareMetal ToolChain multilib layering
ClosedPublic

Authored by michaelplatings on Feb 1 2023, 7:52 AM.

Details

Summary

This enables layering baremetal 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.

Diff Detail

Event Timeline

michaelplatings created this revision.Feb 1 2023, 7:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2023, 7:52 AM
Herald added a subscriber: abidh. · View Herald Transcript
michaelplatings requested review of this revision.Feb 1 2023, 7:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2023, 7:52 AM

No comments on the implementation.

Rebase and remove some braces

PrintArgs -> PrintOptions

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 added inline comments.May 23 2023, 1:34 AM
clang/lib/Driver/ToolChains/BareMetal.cpp
228

Can you use llvm::reverse instead of allocating an entire new collection?

michaelplatings marked an inline comment as done.

Rebase

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

Changed to return an iterator range.

phosek added inline comments.Jun 6 2023, 10:53 PM
clang/lib/Driver/ToolChains/BareMetal.cpp
247–251

What's the reason for returning an empty Multilib, could we instead return and empty range?

Rebase

clang/lib/Driver/ToolChains/BareMetal.cpp
247–251

This is for consistency with the previous behaviour, which would fall back to using a default sysroot if no matching multilib was found.

phosek accepted this revision.Jun 8 2023, 1:13 AM

LGTM

clang/lib/Driver/ToolChains/BareMetal.cpp
241–245

It should be possible to simplify this with llvm::reverse.

249–251

It should be possible to simplify this with llvm::reverse.

clang/lib/Driver/ToolChains/BareMetal.h
75

I'd define an alias for this type with using to avoid repeating it multiple times.

Simplify code as suggested by @phosek

michaelplatings marked 4 inline comments as done.Jun 8 2023, 5:25 AM
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.