Track barrier effects with EffectAnalyzer
diff --git a/2 b/2 deleted file mode 100644 index 08ecfda..0000000 --- a/2 +++ /dev/null
@@ -1,5 +0,0 @@ -Track interfering effects - -JJ: Change ID: nlwsntpx -JJ: -JJ: Lines starting with "JJ:" (like this one) will be removed.
diff --git a/src/ir/effects.cpp b/src/ir/effects.cpp index 2f9dbdd..a3ce1a9 100644 --- a/src/ir/effects.cpp +++ b/src/ir/effects.cpp
@@ -19,7 +19,7 @@ namespace std { -std::ostream& operator<<(std::ostream& o, wasm::EffectAnalyzer& effects) { +std::ostream& operator<<(std::ostream& o, const wasm::EffectAnalyzer& effects) { o << "EffectAnalyzer {\n"; if (effects.branchesOut) { o << "branchesOut\n";
diff --git a/src/ir/effects.h b/src/ir/effects.h index 5d96f72..7841ad1 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h
@@ -1431,6 +1431,97 @@ assert(!transfersControlFlow()); } + int count() const { + size_t count = 0; + auto& effects = *this; + + if (effects.branchesOut) + count++; + if (effects.calls) + count++; + + // Size-based effects (counted as 1 if not empty) + if (!effects.localsRead.empty()) + count++; + if (!effects.localsWritten.empty()) + count++; + if (!effects.mutableGlobalsRead.empty()) + count++; + if (!effects.globalsWritten.empty()) + count++; + + if (effects.readsMemory) + count++; + if (effects.writesMemory) + count++; + if (effects.readsSharedMemory) + count++; + if (effects.writesSharedMemory) + count++; + if (effects.readsTable) + count++; + if (effects.writesTable) + count++; + if (effects.readsMutableStruct) + count++; + if (effects.writesStruct) + count++; + if (effects.readsSharedMutableStruct) + count++; + if (effects.writesSharedStruct) + count++; + if (effects.readsMutableArray) + count++; + if (effects.writesArray) + count++; + if (effects.readsSharedMutableArray) + count++; + if (effects.writesSharedArray) + count++; + if (effects.trap) + count++; + if (effects.implicitTrap) + count++; + + // Order-based effects + if (effects.readOrder != wasm::MemoryOrder::Unordered) + count++; + if (effects.writeOrder != wasm::MemoryOrder::Unordered) + count++; + + if (effects.throws_) + count++; + if (effects.tryDepth) + count++; + if (effects.catchDepth) + count++; + if (effects.danglingPop) + count++; + if (effects.mayNotReturn) + count++; + if (effects.hasReturnCallThrow) + count++; + + // Method-based checks + // if (effects.accessesLocal()) count++; + // if (effects.accessesMutableGlobal()) count++; + // if (effects.accessesMemory()) count++; + // if (effects.accessesTable()) count++; + // if (effects.accessesMutableStruct()) count++; + // if (effects.accessesArray()) count++; + // if (effects.throws()) count++; + // if (effects.transfersControlFlow()) count++; + // if (effects.writesGlobalState()) count++; + // if (effects.readsMutableGlobalState()) count++; + // if (effects.hasNonTrapSideEffects()) count++; + // if (effects.hasSideEffects()) count++; + // if (effects.hasUnremovableSideEffects()) count++; + // if (effects.hasAnything()) count++; + // if (effects.hasExternalBreakTargets()) count++; + + return count; + } + private: void post() { assert(tryDepth == 0); @@ -1460,7 +1551,7 @@ } // namespace wasm namespace std { -std::ostream& operator<<(std::ostream& o, wasm::EffectAnalyzer& effects); +std::ostream& operator<<(std::ostream& o, const wasm::EffectAnalyzer& effects); } // namespace std #endif // wasm_ir_effects_h
diff --git a/src/passes/DeadStoreElimination.cpp b/src/passes/DeadStoreElimination.cpp index 5217f77..ae8e14f 100644 --- a/src/passes/DeadStoreElimination.cpp +++ b/src/passes/DeadStoreElimination.cpp
@@ -54,11 +54,34 @@ // duplicate store, and this would definitely be dead if it weren't for these // effects (AND it's possible that there are conflicting gets as well). std::optional<EffectAnalyzer> conflictingEffects = std::nullopt; -}; -enum class Barrier { - None, - Branch, + friend std::ostream& operator<<(std::ostream& os, const StoreInfo& info) { + os << "StoreInfo { "; + + // Handle the pointer to StructSet + os << "store: "; + if (info.store) { + os << *info.store; + } else { + os << "nullptr"; + } + + os << ", duplicateStores: " << info.duplicateStores; + os << ", conflictingGets: " << info.conflictingGets; + + // Handle the std::optional EffectAnalyzer + os << ", conflictingEffects: "; + if (info.conflictingEffects.has_value()) { + os << *info.conflictingEffects; + } else { + os << "none"; + } + + os << " }"; + return os; + } + + // } }; using Info = std::variant<StoreInfo, EffectAnalyzer>; @@ -102,6 +125,7 @@ barriers.mergeIn(*barrier); } } + std::cout << "Should have got here twice\n"; storeInfos.push_back(StoreInfo{structSet}); } else if (const StructGet* structGet = inst->dynCast<StructGet>()) { for (auto it = storeInfos.rbegin(); it != storeInfos.rend(); ++it) { @@ -123,8 +147,15 @@ getPassOptions(), *module, const_cast<Expression*>(inst)); // Add all the possible effects here // Maybe prune the ones that matter from effects - if (effects.branchesOut || effects.calls || effects.throws() || (!getPassOptions().trapsNeverHappen && effects.trap)) { - storeInfos.push_back(effects); + if (effects.branchesOut || effects.calls || effects.throws() || + (!getPassOptions().trapsNeverHappen && effects.trap)) { + ShallowEffectAnalyzer prunedEffects(getPassOptions(), *module); + prunedEffects.branchesOut = effects.branchesOut; + prunedEffects.calls = effects.calls; + prunedEffects.throws_ = effects.throws_; + prunedEffects.delegateTargets = effects.delegateTargets; + // trap left out because we're using TNH in practice for now + storeInfos.push_back(prunedEffects); } } } @@ -136,10 +167,16 @@ } auto& storeInfo = std::get<StoreInfo>(info); - + + std::cout << storeInfo << "\n"; + + // When running on the small binary and adding a throw, we can tell that + // the store is not dead but the effects don't print out for some reason. if (storeInfo.conflictingEffects) { std::lock_guard _(m); - std::cout<<*const_cast<EffectAnalyzer*>(&*storeInfo.conflictingEffects); + std::cout << "??\n"; + // std::cout<<*const_cast<EffectAnalyzer*>(&*storeInfo.conflictingEffects); + std::cout << *storeInfo.conflictingEffects; } // if (storeInfo.duplicateStores && !storeInfo.conflictingGets && // !storeInfo.conflictingEffects) {
diff --git a/src/passes/GlobalEffects.cpp b/src/passes/GlobalEffects.cpp index fbfb278..cf716a5 100644 --- a/src/passes/GlobalEffects.cpp +++ b/src/passes/GlobalEffects.cpp
@@ -84,7 +84,7 @@ // worst. To do so, clear the effects, which indicates nothing // is known (so anything is possible). // TODO: We could group effects by function type etc. - funcInfo.effects.reset(); + // funcInfo.effects.reset(); } else { // No call here, but update throwing if we see it. (Only do so, // however, if we have effects; if we cleared it - see before -
diff --git a/src/wasm.h b/src/wasm.h index 6027cd0..e32acbd 100644 --- a/src/wasm.h +++ b/src/wasm.h
@@ -818,7 +818,7 @@ } // Print the expression to stderr. Meant for use while debugging. - void dump() const; + void dump(std::ostream& o = std::cout) const; }; const char* getExpressionName(Expression* curr);
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp index a80015a..85b9d50 100644 --- a/src/wasm/wasm.cpp +++ b/src/wasm/wasm.cpp
@@ -124,7 +124,7 @@ // Expressions -void Expression::dump() const { std::cout << *this << '\n'; } +void Expression::dump(std::ostream& o) const { o << *this << '\n'; } const char* getExpressionName(Expression* curr) { switch (curr->_id) {
diff --git a/test-dse.wat b/test-dse.wat index 6a40827..6f36f09 100644 --- a/test-dse.wat +++ b/test-dse.wat
@@ -1,7 +1,11 @@ (module (type $s (struct (field (mut i32)))) - (func $asdf (param $ref (ref $s)) + (tag $t) + (func $asdf (param $ref (ref $s)) (param $b i32) (struct.set $s 0 (local.get $ref) (i32.const 1)) + + (if (local.get $b) (then (throw $t))) + (struct.set $s 0 (local.get $ref) (i32.const 2)) ) )
diff --git a/test-out.wat b/test-out.wat index de30611..c246539 100644 --- a/test-out.wat +++ b/test-out.wat
@@ -1,18 +1,22 @@ (module (type $s (struct (field (mut i32)))) - (type $1 (func (param (ref $s)))) - (func $a (type $1) (param $ref (ref $s)) + (type $1 (func)) + (type $2 (func (param (ref $s) i32))) + (tag $t (type $1)) + (func $asdf (type $2) (param $ref (ref $s)) (param $b i32) (struct.set $s 0 (local.get $ref) - (i32.const 5) + (i32.const 1) + ) + (if + (local.get $b) + (then + (throw $t) + ) ) (struct.set $s 0 (local.get $ref) - (i32.const 4) - ) - (struct.set $s 0 - (local.get $ref) - (i32.const 4) + (i32.const 2) ) ) )