[clang] [Clang] Bring initFeatureMap back to AArch64TargetInfo. (PR #96832)

Tomas Matheson via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 1 04:26:07 PDT 2024


tmatheson-arm wrote:

Thank you for the example, I understand what is happening how.

- Before #94279, we used to add CPU features in `AArch64::initFeatureMap`.
- In #94279, we decided that actually you should do that in the Driver, which should put all `-target-features` it wants on the -cc1 command line. The Driver is responsible for expanding the CPU (and architecture) feature dependencies and their interaction with any modifiers on the command line.
- This means that when `initFeatureMap` runs in `clang`, `FeaturesAsWritten` is populated with the CPU features and is used to initialise the `FeatureMap`.
- In contrast, you are not using the `Driver`, and do not populate `FeaturesAsWritten` with the CPU features.
- Instead, you expect `initFeatureMap` to add CPU features. This is not unreasonable, given that the CPU is passed the function and several other backends add CPU features at this stage.

[This bit of code](https://github.com/llvm/llvm-project/pull/94279/files#diff-2ccae12096c75c4b8422ea0d2fdf6b195896d2554d62cce604e8fcb56a78ef62L1057-L1067) used to crudely add the CPU features to the end of the feature list. However there are some problems with that approach, which we attempted to rectify in #94279:
- CPU features that were explicitly disabled on the command line could actually end up enabled in the backend
- The architecture features (i.e. implied by `-march`) were not treated the same way as the CPU features (`-mcpu`)

For example, if you wrote: `clang -mcpu=cortex-a75+norcpc -###`, you would see all the Cortex-A75 features expanded on the `-cc1` command line, but with RCPC disabled: `-target-feature -rcpc`. But in this case, `AArch64::initFeatureMap` would have re-added `+rcpc`, overriding the command line. (This is technically not the case after [this line](https://github.com/llvm/llvm-project/pull/94279/files#diff-2ccae12096c75c4b8422ea0d2fdf6b195896d2554d62cce604e8fcb56a78ef62L1092) was added, but the general point is that `initFeatureMap` broke feature dependency resolution in ways that are difficult to reason about).

There doesn't seem to be a way to specify an architecture in `TargetOptions`, which looks odd to me. That means there is no way to select e.g. `armv9.4-a` in your example, except by manually adding the features in `TargetOptions::Features` or `TargetOptions::FeaturesAsWritten`.

So the way that we set up the AArch64 backend in #94279 is to require you to calculate your feature set up front, which are then trivially passed through by the default `TargetInfo::initFeatureMap`.

I'm not sure there is a clear answer on this one. I can't see a way to easily let `AArch64:: initFeatureMap` add CPU features again without breaking the dependency resolution. I am open to suggestions though.

If you wanted to go the route of building the feature list before calling `initFeatureMap`, the functions `tools::getTargetFeatures` and `aarch64::getAArch64TargetFeatures` can do that for you. Currently they require a `const Driver &D`, but fundamentally I think they just need a `DiagnosticsEngine&` so that could be changed.

I'm open to other suggestions too.


https://github.com/llvm/llvm-project/pull/96832


More information about the cfe-commits mailing list