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

[weiwei chen via cfe-commits]

weiweichen wrote:

> Thank you for the example, I understand what is happening how.
> 
> * Before [[AArch64] Decouple feature dependency expansion. #94279](https://github.com/llvm/llvm-project/pull/94279), we used to add CPU features in `AArch64::initFeatureMap`.
> * In [[AArch64] Decouple feature dependency expansion. #94279](https://github.com/llvm/llvm-project/pull/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.

Oh, thank you so much for the explanation and suggestions on the APIs to try! We'll play with the APIs to see if we can get a cleaner workaround on our side. On the other hand, as for the clang API's perspective, maybe it's ok that different backends have different behavior in terms how they retrieve target features or is there room to improve for consistency as well?

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