[PATCH] D140959: RFC: Multilib prototype

Mon Jan 23 04:00:19 PST 2023

lenary added a comment.

Thanks for putting together this proposal.

Broadly, I think it's good, and I think it solves a lot of rough edges that I recall running into with the GCC multilib configuration files in the past.

I do worry about using `-cc1` args. On the one hand, it's the best place for getting access to information about e.g. exceptions, floating point abi, and other concepts (optimisation level?), but on the other hand, it is a reasonably unstable interface, and it's parsed in a complex and stateful way -- later options can override earlier ones -- which makes it difficult to parse with regular expressions.

To be clear: I don't think there's a better way of handling exceptions/no-exceptions library selection (than parsing the `-cc1` arguments) or the floating point abi library selection, so having a way to examining the `-cc1` arguments is a necessary "lowest common denominator" interface (I'll come back to this later), but I think there are places we can do better.

In terms of finding a more stable interface, the Arm ACLE recently added Beta support for Function Multi-Versioning <https://github.com/ARM-software/acle/blob/main/main/acle.md#function-multi-versioning> which is proposing a lot of stable identifiers for architectural features in v8-a and v8-r onwards. These names are expected to be written by programmers in their source code, so are a stable interface in a way that `-target-feature +<name>` is absolutely not. These are resolved by clang in order to handle duplicating the same function to compile it for multiple targets, which to my mind is the exact same problem that multi-libs are trying to solve. We have specified stable names for a lot of features in the ACLE (and presumably will continue to do so), as well as a priority system for resolving how to choose which function should be used at runtime. I concede that these stable names do not cover M-profile or pre-v8 architectures, which are important to embedded toolchains.

The information about these stable feature names is known about by `AArch64TargetInfo` right now, and we are looking at where exactly this information should be best understood by LLVM in general (maybe the `TargetParser` library). Given where it is in clang's code, I think it should be usable by both the driver and internally inside `-cc1`, but I'm not 100% sure the C++ interface is perfect for both. But, we can always revisit the C++ interface to make it work for what we want.

So, to concretely turn this into a proposal, I think there are two things you could do:

- in the `arguments:` part of the yaml structure, you currently have `regex:` which is for matching on the `-cc1` arguments, we could have a `archFeatures:` for matching on a list/set of named features (instead of the `regex:`), to turn them into multilib library attributes.
- treat the stable architectural feature names like extra implicit attributes that don't show up in the `arguments:` section, but can be used in the `variants:` section like the `-cc1`-derived attributes. Here, we would need to be careful the architectural feature names didn't clash with the user-provided names from the `arguments:` section, and we might need to have a way to specify the absence of architectural features as well. I'm not 100% convinced that the implicit-ness of this option is good, but I think the stability wins are useful.

To cover some drawbacks of the approach of using these stable names:

- Not all targets support function multi-versioning. In this case, they are welcome to fall back to using `-cc1` arguments, as a lowest common denominator approach.
- Not all multilibs are chosen based on architectural features: again, you can fall-back to `-cc1` arguments, especially as I think e.g. `-fno-exceptions` is more stable than `-target-feature +<name>`.
- Even for Arm/AArch64, there are pre-v8-a/v8-r architectures which don't have stable names in the ACLE yet. Again, in the short term we could use `-cc1` arguments, before we stabilise architectural names.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140959/new/

https://reviews.llvm.org/D140959