Page MenuHomePhabricator
Differential D143059

[Driver] Enable selecting multiple multilibs
AcceptedPublic
Actions

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

Details

Reviewers
phosek
peter.smith
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
View All

TimeTest
120 msx64 debian > Clang.Driver::mips-mti-linux.c
Script: -- : 'RUN: at line 10'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -### /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/mips-mti-linux.c 2>&1 --target=mips-mti-linux -mips32r2 -mhard-float -no-pie -rtlib=platform -fuse-ld=ld --sysroot=/var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/Inputs/mips_mti_linux/sysroot | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=CHECK-BE-HF-32R2 /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/mips-mti-linux.c

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
Herald added subscribers: luke, abrachet, frasercrmck and 22 others. · View Herald Transcript
michaelplatings requested review of this revision.Feb 1 2023, 3:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2023, 3:29 AM
Herald added subscribers: cfe-commits, pcwang-thead, MaskRay. · View Herald Transcript
michaelplatings added a parent revision: D142986: Enable multilib.yaml in the BareMetal ToolChain.Feb 1 2023, 3:30 AM
Herald added a subscriber: ormris. · View Herald TranscriptFeb 1 2023, 3:30 AM
Harbormaster completed remote builds in B211189: Diff 493892.Feb 1 2023, 5:18 AM
michaelplatings added a child revision: D143075: BareMetal ToolChain multilib layering.Feb 1 2023, 7:52 AM
michaelplatings updated this revision to Diff 494262.Feb 2 2023, 4:57 AM

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

Harbormaster completed remote builds in B211459: Diff 494262.Feb 2 2023, 7:42 AM
michaelplatings updated this revision to Diff 494416.Feb 2 2023, 12:48 PM

Rebase

Harbormaster completed remote builds in B211567: Diff 494416.Feb 2 2023, 3:06 PM
amilendra added a subscriber: amilendra.Feb 2 2023, 9:27 PM
michaelplatings updated this revision to Diff 495072.Feb 6 2023, 3:42 AM

Rebase

Harbormaster completed remote builds in B212055: Diff 495072.Feb 6 2023, 3:43 AM
michaelplatings updated this revision to Diff 495464.Feb 7 2023, 3:53 AM

Rebase

Herald added a subscriber: jobnoorman. · View Herald TranscriptFeb 7 2023, 3:53 AM
Harbormaster completed remote builds in B212337: Diff 495464.Feb 7 2023, 5:55 AM
michaelplatings mentioned this in D142905: [Driver] Change multilib selection algorithm.Feb 7 2023, 7:25 AM
michaelplatings updated this revision to Diff 495558.Feb 7 2023, 8:34 AM

Rebase

Harbormaster completed remote builds in B212410: Diff 495558.Feb 7 2023, 11:43 AM
peter.smith added a subscriber: peter.smith.Feb 10 2023, 4:13 AM

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

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

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
319

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 updated this revision to Diff 496538.Feb 10 2023, 10:08 AM

Rebase

Harbormaster completed remote builds in B213106: Diff 496538.Feb 10 2023, 11:47 AM
michaelplatings updated this revision to Diff 499500.Feb 22 2023, 7:17 AM
michaelplatings marked 2 inline comments as done.

Rebase and apply changes requested by @peter.smith

Harbormaster completed remote builds in B215253: Diff 499500.Feb 22 2023, 8:02 AM
michaelplatings retitled this revision from [NFC] Enable selecting multiple multilibs to [Driver] Enable selecting multiple multilibs.Feb 22 2023, 8:15 AM
michaelplatings updated this revision to Diff 503904.Thu, Mar 9, 1:07 PM

flags -> tags

Harbormaster completed remote builds in B218496: Diff 503904.Thu, Mar 9, 3:18 PM
peter.smith accepted this revision.Mon, Mar 13, 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.Mon, Mar 13, 9:26 AM
michaelplatings updated this revision to Diff 504774.Mon, Mar 13, 11:29 AM

Rebase

Harbormaster completed remote builds in B219118: Diff 504774.Mon, Mar 13, 2:43 PM
michaelplatings updated this revision to Diff 505051.Tue, Mar 14, 5:42 AM

tags->flags

Harbormaster completed remote builds in B219323: Diff 505051.Tue, Mar 14, 7:40 AM
michaelplatings updated this revision to Diff 505288.Tue, Mar 14, 3:10 PM

Rebase

Harbormaster completed remote builds in B219495: Diff 505288.Tue, Mar 14, 4:46 PM
michaelplatings updated this revision to Diff 508039.Fri, Mar 24, 3:58 AM

Rebase

Harbormaster completed remote builds in B221544: Diff 508039.Sat, Mar 25, 9:37 AM

Revision Contents
Changeset List

PathSize
clang/
include/
clang/
Driver/
8 lines
6 lines
lib/
Driver/
15 lines
19 lines
4 lines
ToolChains/
16 lines
11 lines
11 lines
4 lines
45 lines
4 lines
2 lines
6 lines
2 lines
7 lines
4 lines
test/
Driver/
11 lines
unittests/
Driver/
21 lines
64 lines

Diff 508039

View Options

clang/include/clang/Driver/Multilib.h

Loading...
View Options

clang/include/clang/Driver/ToolChain.h

Loading...
View Options

clang/lib/Driver/Driver.cpp

Loading...
View Options

clang/lib/Driver/Multilib.cpp

Loading...
View Options

clang/lib/Driver/ToolChain.cpp

Loading...
View Options

clang/lib/Driver/ToolChains/BareMetal.cpp

Loading...
View Options

clang/lib/Driver/ToolChains/CSKYToolChain.cpp

Loading...
View Options

clang/lib/Driver/ToolChains/Fuchsia.cpp

Loading...
View Options

clang/lib/Driver/ToolChains/Gnu.h

Loading...
View Options

clang/lib/Driver/ToolChains/Gnu.cpp

Loading...
View Options

clang/lib/Driver/ToolChains/Hexagon.cpp

Loading...
View Options

clang/lib/Driver/ToolChains/Hurd.cpp

Loading...
View Options

clang/lib/Driver/ToolChains/Linux.cpp

Loading...
View Options

clang/lib/Driver/ToolChains/MipsLinux.cpp

Loading...
View Options

clang/lib/Driver/ToolChains/OHOS.cpp

Loading...
View Options

clang/lib/Driver/ToolChains/RISCVToolchain.cpp

Loading...
View Options

clang/test/Driver/fuchsia.cpp

Loading...
View Options

clang/unittests/Driver/MultilibBuilderTest.cpp

Loading...
View Options

clang/unittests/Driver/MultilibTest.cpp

Loading...