From 2026789785ffaa98e7dbb5bdd5c0f8ae04ce16ac Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 1 Oct 2024 12:43:05 +0200 Subject: [PATCH] [WebAssembly] Fix feature coalescing This fixes a problem introduced in #80094. That PR copied negative features from the TargetMachine to the end of the feature string. This is not correct, because even if we have a baseline TM of say `-simd128`, but a function with `+simd128`, the coalesced feature string must have `+simd128`, not `-sidm128`. To address the original motivation of that PR, we should instead explicitly materialize the negative features in the target feature string, so that explicitly disabled default features are honored. Unfortunately, there doesn't seem to be any way to actually test this using llc, because `-mattr` appends the specified features to the end of the `"target-features"` attribute. I've tested this locally by making it prepend the features instead. --- .../Target/WebAssembly/WebAssemblyTargetMachine.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp index 73765f8fa0092..dcc7b566d9c46 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp @@ -202,8 +202,7 @@ class CoalesceFeaturesAndStripAtomics final : public ModulePass { bool runOnModule(Module &M) override { FeatureBitset Features = coalesceFeatures(M); - std::string FeatureStr = - getFeatureString(Features, WasmTM->getTargetFeatureString()); + std::string FeatureStr = getFeatureString(Features); WasmTM->setTargetFeatureString(FeatureStr); for (auto &F : M) replaceFeatures(F, FeatureStr); @@ -241,17 +240,14 @@ class CoalesceFeaturesAndStripAtomics final : public ModulePass { return Features; } - static std::string getFeatureString(const FeatureBitset &Features, - StringRef TargetFS) { + static std::string getFeatureString(const FeatureBitset &Features) { std::string Ret; for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) { if (Features[KV.Value]) Ret += (StringRef("+") + KV.Key + ",").str(); + else + Ret += (StringRef("-") + KV.Key + ",").str(); } - SubtargetFeatures TF{TargetFS}; - for (std::string const &F : TF.getFeatures()) - if (!SubtargetFeatures::isEnabled(F)) - Ret += F + ","; return Ret; }