Hi Jon, I think when I wrote that I was overly attached to the idea that you should use addMultilibFlag. If you always use addMultilibFlag then that case cannot work, but get a little more creative and I agree you can express it:
auto Soft = MultilibBuilder("/soft","","",0)
.flag("+mfloat-abi=soft");
auto SoftFP = MultilibBuilder("/softfp","","",1)
.flag("+mfloat-abi=softfp");
auto Hard = MultilibBuilder("/hard","","",2)
.flag("+mfloat-abi=hard");
MultilibSet Multilibs = MultilibSet().Either(Soft, SoftFP, Hard);
if (ABI == arm::FloatABI::Soft) {
Flags.push_back("+mfloat-abi=soft");
Flags.push_back("-mfloat-abi=softfp");
Flags.push_back("-mfloat-abi=hard");
}
if (ABI == arm::FloatABI::SoftFP) {
Flags.push_back("+mfloat-abi=soft");
Flags.push_back("+mfloat-abi=softfp");
Flags.push_back("-mfloat-abi=hard");
}
if (ABI == arm::FloatABI::Hard) {
Flags.push_back("-mfloat-abi=soft");
Flags.push_back("-mfloat-abi=softfp");
Flags.push_back("+mfloat-abi=hard");
}
I am aiming to change the selection criterion to “a multilib is compatible its flags are a subset of the flags derived from the command line arguments” and then the code gets smaller:
auto Soft = MultilibBuilder("/soft","","")
.flag("+mfloat-abi=soft");
auto SoftFP = MultilibBuilder("/softfp","","")
.flag("+mfloat-abi=softfp");
auto HardFP = MultilibBuilder("/hard","","")
.flag("+mfloat-abi=hard");
MultilibSet Multilibs = MultilibSet().Either(Soft, SoftFp, HardFp);
if (ABI == arm::FloatABI::Soft)
Flags.push_back("+mfloat-abi=soft");
if (ABI == arm::FloatABI::SoftFP) {
Flags.push_back("+mfloat-abi=soft");
Flags.push_back("+mfloat-abi=softfp");
}
if (ABI == arm::FloatABI::Hard)
Flags.push_back("+mfloat-abi=hard");
We’ve gone live with a version 0.1 of the multilib format in LLVM Embedded Toolchain for Arm 16.0.0. Feedback is very welcome either here or in the GitHub issues.
Thanks, seeing a real-world multilib.yaml example is actually very helpful (including the logic that was used to generate it). The part I’m still unsure about is why do we need PrintOptions separate from Flags. This aspect is adding a lot of implementation and configuration complexity, and it’s not clear to me why it’s needed. Can you explain the motivation? Why can’t we use Flags for the -print-multi-lib output?
Unlike regular options like -fno-exceptions and -mfloat-abi which exist as independent options, all the architecture extensions are crushed into a single option e.g. -march=armv8.1-m.main+dsp+mve.fp+fp.dp+cdecp0+pacbti and each extension can be negated by prefixing it with no.
This is not ideal for multilib. When writing a multilib configuration I want to be able to express “this library requires the DSP extension and the PACBTI extension”. Currently the code in my patches allows me to add march=+dsp and march=+pacbti to multilib.yaml to do that. Neither flag is a valid command line option, and currently that’s OK.
What if I were to change the multilib system to require that flags are valid command line options?
Instead of generating march=+thing flags, a valid normalized -march option would have to be generated, for example -march=thumbv8.1m.main+dsp+mve+fp16+pacbti.
Each library in a multilib configuration would need a regular expression to match any suitable -march option and map it to a specific option e.g.
It’s possible to get this to work, but it’s going to be more error-prone than directly stating the required extensions. It’s also more brittle. Let’s say in a later version of Clang that we change how the architecture extension list is normalised, and we append disabled options to the generated -march option. Now the regular expression in the multilib configuration is broken because we forgot .* at the end. Or maybe the extensions get reordered and now pacbti preceeds dsp. etc.
I suspect this problem will also crop up on RISC-V since that also has ISA extensions concatenated into a single -march string.
However, if it helps move things forwards, I can go the route of generating a normalized -march argument as described above, drop PrintOptions for now, and we can revisit this issue later.
Thanks for the additional context Michael, that’s really helpful. I’d also suggest including some of this in the documentation since it might be helpful in understanding the design.
I like this direction since it would let us land the rest of the stack and then us experiment with other possible approaches.
I agree that relying solely on regular expressions is potentially error-prone, but there might be other ways to handle this case in the flag mapping. For example, this problem reminds me of minion matching in Salt; to handle more complex scenarios Salt supports compound matchers and I wonder if we could use a similar approach here (to express your example you could use something like E@-march=thumbv8\.[1-9]m and M@feature:pacbti and not M@feature:pacbti)? This is just one idea, there might be other solutions.
Hi Petr, I’ve gone ahead and updated the code to generate a normalised -march option and removed the separation of “PrintOptions”. Looking forward to your thoughts on https://reviews.llvm.org/D146757 et al.
The multilib changes were landed last Wednesday and all seems well. Thanks @petrhosek for approving the changes. It was a long slog so thanks for keeping going with it.
LLVM Embedded Toolchain for Arm now has multilib support in its main branch. I’ll be interested to see if/how the configurable multilib can be used in other projects.
The feedback we’ve had so far is that it’s very hard to debug multilib when the user’s options don’t match any multilib variant. Therefore I’m working on making the multilib system emit an error message in this case, to hopefully direct the user to being able to find a suitable set of compile flags. I should have a patch up for review soon, and I hope to also get it in before LLVM 17 branching begins. Edit: patch is ⚙ D153292 [Driver][BareMetal] Error if no matching multilib