From d444ab483b2fcfb6452439e3703089640c0ead49 Mon Sep 17 00:00:00 2001 From: Benoit Jacob Date: Mon, 3 Feb 2025 23:39:46 -0500 Subject: [PATCH] [CPU] Aarch64: actually do `reserve-x18` (#19895) 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 --- .../target/LLVMCPU/ResolveCPUAndCPUFeatures.cpp | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/compiler/plugins/target/LLVMCPU/ResolveCPUAndCPUFeatures.cpp b/compiler/plugins/target/LLVMCPU/ResolveCPUAndCPUFeatures.cpp index 5850be4babba..628b042b95b1 100644 --- a/compiler/plugins/target/LLVMCPU/ResolveCPUAndCPUFeatures.cpp +++ b/compiler/plugins/target/LLVMCPU/ResolveCPUAndCPUFeatures.cpp @@ -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(); } }