Skip to content

Commit

Permalink
[CPU] Aarch64: actually do reserve-x18 (#19895)
Browse files Browse the repository at this point in the history
The code was intending to add the `reserve-x18` flag, and it was being
executed... but the `hasFlag` method that it was calling wasn't doing
that it thought it was.

That `hasFlag` method is just a static member that returns `true` if its
string argument starts with a `+`.

There isn't an actual method on `SubtargetFeatures` to check if we
already have a given feature. Anyway, it doesn't matter - in this case,
we don't already have the feature, and even if we did, multiply
specified features are not a problem.

But it really, really matters that we don't accidentally allocate `x18`
again :-)

Fixes #19873.

---------

Signed-off-by: Benoit Jacob <[email protected]>
  • Loading branch information
bjacob authored Feb 4, 2025
1 parent 4fffb0e commit d444ab4
Showing 1 changed file with 1 addition and 9 deletions.
10 changes: 1 addition & 9 deletions compiler/plugins/target/LLVMCPU/ResolveCPUAndCPUFeatures.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,8 @@ void tweakCPUFeatures(const llvm::Triple &triple, std::string &cpu,
std::string &cpuFeatures) {
if (triple.isAArch64()) {
llvm::SubtargetFeatures targetCpuFeatures(cpuFeatures);
// Helper to add a feature if not already present. This check matters as
// we check for equality of features to tell whether to generate the error
// about implicitly targeting a generic CPU.
auto addFeature = [&](const char *feature) {
if (!targetCpuFeatures.hasFlag(std::string("+") + feature)) {
targetCpuFeatures.AddFeature(feature, true);
}
};
// x18 is platform-reserved per the Aarch64 procedure call specification.
addFeature("reserve-x18");
targetCpuFeatures.AddFeature("reserve-x18", true);
cpuFeatures = targetCpuFeatures.getString();
}
}
Expand Down

0 comments on commit d444ab4

Please sign in to comment.