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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to