diff options
author | Alon Zakai <azakai@google.com> | 2022-03-31 11:10:14 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-03-31 18:10:14 +0000 |
commit | f7052ea934058339662e34e3472090df48a377e3 (patch) | |
tree | 539756a156dbfb606503311ec6203b4d2887646d | |
parent | 33fe4f12bd30739790da3d34f5fae844f47a327f (diff) | |
download | binaryen-f7052ea934058339662e34e3472090df48a377e3.tar.gz binaryen-f7052ea934058339662e34e3472090df48a377e3.tar.bz2 binaryen-f7052ea934058339662e34e3472090df48a377e3.zip |
[NFC] Refactor Feature::All to match FeatureSet.setAll() (#4557)
As we recently noted in #4555, that Feature::All and FeatureSet.setAll()
are different is potentially confusing...
I think the best thing is to make them identical. This does that, and adds a
new Feature::AllPossible which is everything possible and not just the
set of all features that are enabled by -all.
This undoes part of #4555 as now the old/simpler code works properly.
-rw-r--r-- | src/tools/wasm-shell.cpp | 6 | ||||
-rw-r--r-- | src/wasm-features.h | 16 | ||||
-rw-r--r-- | test/binaryen.js/kitchen-sink.js.txt | 2 | ||||
-rw-r--r-- | test/example/c-api-kitchen-sink.txt | 2 |
4 files changed, 15 insertions, 11 deletions
diff --git a/src/tools/wasm-shell.cpp b/src/tools/wasm-shell.cpp index 2d0fbf14c..0de93c69e 100644 --- a/src/tools/wasm-shell.cpp +++ b/src/tools/wasm-shell.cpp @@ -134,7 +134,7 @@ protected: lastModule = module->name; builders[moduleName] = builder; modules[moduleName].swap(module); - modules[moduleName]->features.setAll(); + modules[moduleName]->features = FeatureSet::All; bool valid = WasmValidator().validate(*modules[moduleName]); if (!valid) { std::cout << *modules[moduleName] << '\n'; @@ -237,7 +237,7 @@ protected: void parseModuleAssertion(Element& s) { Module wasm; - wasm.features.setAll(); + wasm.features = FeatureSet::All; std::unique_ptr<SExpressionWasmBuilder> builder; auto id = s[0]->str(); @@ -358,7 +358,7 @@ protected: "memory", spectest->memory.name, ExternalKind::Memory)); modules["spectest"].swap(spectest); - modules["spectest"]->features.setAll(); + modules["spectest"]->features = FeatureSet::All; instantiate(modules["spectest"].get()); linkedInstances["spectest"] = instances["spectest"]; // print_* functions are handled separately, no need to define here. diff --git a/src/wasm-features.h b/src/wasm-features.h index df4397e78..64d831ecc 100644 --- a/src/wasm-features.h +++ b/src/wasm-features.h @@ -43,7 +43,11 @@ struct FeatureSet { GCNNLocals = 1 << 13, RelaxedSIMD = 1 << 14, ExtendedConst = 1 << 15, - All = (1 << 16) - 1 + // GCNNLocals are opt-in: merely asking for "All" does not apply them. To + // get all possible values use AllPossible. See setAll() below for more + // details. + All = ((1 << 16) - 1) & ~GCNNLocals, + AllPossible = (1 << 16) - 1, }; static std::string toString(Feature f) { @@ -88,7 +92,7 @@ struct FeatureSet { std::string toString() const { std::string ret; uint32_t x = 1; - while (x & Feature::All) { + while (x & Feature::AllPossible) { if (features & x) { if (!ret.empty()) { ret += ", "; @@ -126,7 +130,7 @@ struct FeatureSet { bool hasGCNNLocals() const { return (features & GCNNLocals) != 0; } bool hasRelaxedSIMD() const { return (features & RelaxedSIMD) != 0; } bool hasExtendedConst() const { return (features & ExtendedConst) != 0; } - bool hasAll() const { return (features & All) != 0; } + bool hasAll() const { return (features & AllPossible) != 0; } void set(FeatureSet f, bool v = true) { features = v ? (features | f) : (features & ~f); @@ -161,17 +165,17 @@ struct FeatureSet { // --enable-gc-nn-locals -all work (that is, if we enable the feature, // then -all does not disable it; it simply does not enable it by itself). auto oldGCNNLocals = hasGCNNLocals(); - features = All; + features = AllPossible; setGCNNLocals(oldGCNNLocals); } void enable(const FeatureSet& other) { features |= other.features; } void disable(const FeatureSet& other) { - features = features & ~other.features & All; + features = features & ~other.features & AllPossible; } template<typename F> void iterFeatures(F f) const { - for (uint32_t feature = MVP + 1; feature < All; feature <<= 1) { + for (uint32_t feature = MVP + 1; feature < AllPossible; feature <<= 1) { if (has(feature)) { f(static_cast<Feature>(feature)); } diff --git a/test/binaryen.js/kitchen-sink.js.txt b/test/binaryen.js/kitchen-sink.js.txt index 376eee7f0..3ea3f11a9 100644 --- a/test/binaryen.js/kitchen-sink.js.txt +++ b/test/binaryen.js/kitchen-sink.js.txt @@ -44,7 +44,7 @@ Features.Memory64: 2048 Features.TypedFunctionReferences: 4096 Features.RelaxedSIMD: 16384 Features.ExtendedConst: 32768 -Features.All: 65535 +Features.All: 57343 InvalidId: 0 BlockId: 1 IfId: 2 diff --git a/test/example/c-api-kitchen-sink.txt b/test/example/c-api-kitchen-sink.txt index f759864a8..95d1a1ed6 100644 --- a/test/example/c-api-kitchen-sink.txt +++ b/test/example/c-api-kitchen-sink.txt @@ -28,7 +28,7 @@ BinaryenFeatureMemory64: 2048 BinaryenFeatureTypedFunctionReferences: 4096 BinaryenFeatureRelaxedSIMD: 16384 BinaryenFeatureExtendedConst: 32768 -BinaryenFeatureAll: 65535 +BinaryenFeatureAll: 57343 (f32.neg (f32.const -33.61199951171875) ) |