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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
