Skip to content

Commit

Permalink
[WebAssembly] Fix feature coalescing (llvm#110647)
Browse files Browse the repository at this point in the history
This fixes a problem introduced in llvm#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 should have
`+simd128`, not `-simd128`.

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.
  • Loading branch information
nikic authored and bricknerb committed Oct 17, 2024
1 parent beeadcd commit 06d34b3
Showing 1 changed file with 4 additions and 8 deletions.
12 changes: 4 additions & 8 deletions llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,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);
Expand Down Expand Up @@ -244,17 +243,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;
}

Expand Down

0 comments on commit 06d34b3

Please sign in to comment.