Merge remote-tracking branch 'origin/main' into gcrotr
diff --git a/src/ir/properties.cpp b/src/ir/properties.cpp
index b5d2df9..d52445b 100644
--- a/src/ir/properties.cpp
+++ b/src/ir/properties.cpp
@@ -36,4 +36,36 @@
return scanner.generative;
}
+Expression* getSingleDescendantWithEffects(Expression* curr, const PassOptions& options, Module& module) {
+ struct Scanner : public PostWalker<Scanner, UnifiedExpressionVisitor<Scanner>> {
+ const PassOptions& options;
+ Module& module;
+
+ Scanner(const PassOptions& options, Module& module) : options(options), module(module) {}
+
+ Expression* withEffects = nullptr;
+ bool multiple = false;
+
+ void visitExpression(Expression* curr) {
+ if (multiple) {
+ return;
+ }
+ // This handles simple patterns, but cannot ignore nested things that can
+ // be ignored as a whole, which the SimplifyGlobals readOnlyToWrite code
+ // might be able to.
+ ShallowEffectAnalyzer effects(options, module, curr);
+ if (effects.hasSideEffects() || effects.readsMutableGlobalState()) {
+ if (withEffects) {
+ multiple = true;
+ withEffects = nullptr;
+ } else {
+ withEffects = curr;
+ }
+ }
+ }
+ } scanner(options, module);
+ scanner.walk(curr);
+ return scanner.withEffects;
+}
+
} // namespace wasm::Properties
diff --git a/src/ir/properties.h b/src/ir/properties.h
index d2c5aff..f58be83 100644
--- a/src/ir/properties.h
+++ b/src/ir/properties.h
@@ -414,6 +414,8 @@
//
bool isGenerative(Expression* curr, FeatureSet features);
+Expression* getSingleDescendantWithEffects(Expression* curr, const PassOptions& options, Module& module);
+
} // namespace wasm::Properties
#endif // wasm_ir_properties_h
diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h
index f686923..8470ecf 100644
--- a/src/ir/struct-utils.h
+++ b/src/ir/struct-utils.h
@@ -106,7 +106,7 @@
//
// * Note an expression written into a field.
//
-// void noteExpression(Expression* expr, HeapType type, Index index, T& info);
+// void noteWrite(Expression* expr, HeapType type, Index index, T& info);
//
// * Note a default value written during creation.
//
@@ -135,6 +135,11 @@
: functionNewInfos(functionNewInfos),
functionSetGetInfos(functionSetGetInfos) {}
+ // We queue reads here. Some of them may be ignored later if they are part of
+ // a copy or a readOnlyToWrite pattern (in which case we only emit that
+ // particular pattern for the entire thing).
+ std::unordered_set<StructGet*> reads;
+
void visitStructNew(StructNew* curr) {
auto type = curr->type;
if (type == Type::unreachable) {
@@ -150,7 +155,7 @@
static_cast<SubType*>(this)->noteDefault(
fields[i].type, heapType, i, infos[i]);
} else {
- noteExpressionOrCopy(curr->operands[i], heapType, i, infos[i]);
+ noteWriteOrCopy(curr->operands[i], heapType, i, infos[i]);
}
}
}
@@ -162,7 +167,7 @@
}
// Note a write to this field of the struct.
- noteExpressionOrCopy(curr->value,
+ noteWriteOrCopy(curr->value,
type.getHeapType(),
curr->index,
functionSetGetInfos[this->getFunction()]
@@ -175,16 +180,25 @@
return;
}
- auto heapType = type.getHeapType();
- auto index = curr->index;
- static_cast<SubType*>(this)->noteRead(
- heapType,
- index,
- functionSetGetInfos[this->getFunction()][heapType][index]);
+ reads.insert(curr);
+ }
+
+ void visitFunction(Function* curr) {
+ // "Flush" any reads that have not been removed (which they would have had
+ // they been part of a copy of readOnlyToWrite pattern).
+ for (auto* read : reads) {
+ auto type = read->ref->type;
+ auto heapType = type.getHeapType();
+ auto index = read->index;
+ static_cast<SubType*>(this)->noteRead(
+ heapType,
+ index,
+ functionSetGetInfos[this->getFunction()][heapType][index]);
+ }
}
void
- noteExpressionOrCopy(Expression* expr, HeapType type, Index index, T& info) {
+ noteWriteOrCopy(Expression* expr, HeapType type, Index index, T& info) {
// Look at the value falling through, if it has the exact same type
// (otherwise, we'd need to consider both the type actually written and the
// type of the fallthrough, somehow).
@@ -193,14 +207,48 @@
if (fallthrough->type == expr->type) {
expr = fallthrough;
}
- if (auto* get = expr->dynCast<StructGet>()) {
- if (get->index == index && get->ref->type != Type::unreachable &&
- get->ref->type.getHeapType() == type) {
- static_cast<SubType*>(this)->noteCopy(type, index, info);
+
+ // Check if it is a direct copy (a write of a read from the same field).
+ auto readsSameField = [&](Expression* curr) -> StructGet* {
+ if (auto* get = curr->dynCast<StructGet>()) {
+ if (get->index == index && get->ref->type != Type::unreachable &&
+ get->ref->type.getHeapType() == type) {
+ return get;
+ }
+ }
+ return nullptr;
+ };
+ if (auto* get = readsSameField(expr)) {
+ assert(reads.count(get));
+ reads.erase(get);
+ static_cast<SubType*>(this)->noteCopy(type, index, info);
+ return;
+ }
+
+ // Check if it is a read-only-to-write situation, that is, where we read the
+ // field, process that value, then write that result. This is similar to a
+ // copy that was already handled, but the value may not be identical; still,
+ // it has the property that the read value does not "escape" anywhere but
+ // back into this field. (See SimplifyGlobals for the readOnlyToWrite logic
+ // there that is similar to this. Perhaps we can share some code somehow
+ // eventually TODO For now, handle simple cases like incrementing a field
+ // that we see in practice in j2wasm output.)
+ if (auto* single = Properties::getSingleDescendantWithEffects(expr, this->getPassOptions(), *this->getModule())) {
+ if (auto* get = readsSameField(single)) {
+ assert(reads.count(get));
+ reads.erase(get);
+ static_cast<SubType*>(this)->noteReadOnlyToWrite(expr, type, index, info);
return;
}
}
- static_cast<SubType*>(this)->noteExpression(expr, type, index, info);
+
+ // Otherwise, note a general write.
+ static_cast<SubType*>(this)->noteWrite(expr, type, index, info);
+ }
+
+ void noteReadOnlyToWrite(Expression* expr, HeapType type, Index index, T& info) {
+ // By default call the same code as a general write.
+ static_cast<SubType*>(this)->noteWrite(expr, type, index, info);
}
FunctionStructValuesMap<T>& functionNewInfos;
diff --git a/src/passes/ConstantFieldPropagation.cpp b/src/passes/ConstantFieldPropagation.cpp
index 340b3b9..a381dba 100644
--- a/src/passes/ConstantFieldPropagation.cpp
+++ b/src/passes/ConstantFieldPropagation.cpp
@@ -256,7 +256,7 @@
: StructUtils::StructScanner<PossibleConstantValues, PCVScanner>(
functionNewInfos, functionSetInfos) {}
- void noteExpression(Expression* expr,
+ void noteWrite(Expression* expr,
HeapType type,
Index index,
PossibleConstantValues& info) {
diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp
index e8cee2b..c2e368b 100644
--- a/src/passes/GlobalTypeOptimization.cpp
+++ b/src/passes/GlobalTypeOptimization.cpp
@@ -43,19 +43,26 @@
// Information about usage of a field.
struct FieldInfo {
bool hasWrite = false;
- bool hasRead = false;
+ // Reads noted here are *escaping* reads, since a read that is part of a
+ // copy of readOnlyToWrite pattern would not be recorded.
+ bool hasReads = false;
void noteWrite() { hasWrite = true; }
- void noteRead() { hasRead = true; }
-
+ void noteRead() {
+ hasReads = true;
+ }
+ void noteReadOnlyToWrite() {
+ // Do *not* mark either a read or a write. We can remove this field if all
+ // it has are readOnlyToWrite (or copy) operations.
+ }
bool combine(const FieldInfo& other) {
bool changed = false;
if (!hasWrite && other.hasWrite) {
hasWrite = true;
changed = true;
}
- if (!hasRead && other.hasRead) {
- hasRead = true;
+ if (!hasReads && other.hasReads) {
+ hasReads = true;
changed = true;
}
return changed;
@@ -74,7 +81,7 @@
: StructUtils::StructScanner<FieldInfo, FieldInfoScanner>(
functionNewInfos, functionSetGetInfos) {}
- void noteExpression(Expression* expr,
+ void noteWrite(Expression* expr,
HeapType type,
Index index,
FieldInfo& info) {
@@ -87,7 +94,14 @@
}
void noteCopy(HeapType type, Index index, FieldInfo& info) {
- info.noteWrite();
+ info.noteReadOnlyToWrite();
+ }
+
+ void noteReadOnlyToWrite(Expression* expr,
+ HeapType type,
+ Index index,
+ FieldInfo& info) {
+ info.noteReadOnlyToWrite();
}
void noteRead(HeapType type, Index index, FieldInfo& info) {
@@ -178,13 +192,13 @@
// Process removability. First, see if we can remove anything before we
// start to allocate info for that.
if (std::any_of(infos.begin(), infos.end(), [&](const FieldInfo& info) {
- return !info.hasRead;
+ return !info.hasReads;
})) {
auto& indexesAfterRemoval = indexesAfterRemovals[type];
indexesAfterRemoval.resize(fields.size());
Index skip = 0;
for (Index i = 0; i < fields.size(); i++) {
- if (infos[i].hasRead) {
+ if (infos[i].hasReads) {
indexesAfterRemoval[i] = i - skip;
} else {
indexesAfterRemoval[i] = RemovedField;
@@ -346,8 +360,26 @@
return;
}
- auto newIndex = getNewIndex(curr->ref->type.getHeapType(), curr->index);
- if (newIndex != RemovedField) {
+ auto heapType = curr->ref->type.getHeapType();
+ auto newIndex = getNewIndex(heapType, curr->index);
+
+ // Map to the new index if this is a struct.set that should remain, that
+ // is, if the field remains present, and if we are not making it
+ // immutable (if a struct.set exists and we are in fact making this
+ // field immutable, that means that all writes to it are just copies or
+ // read-only-to-write things, that is, the writes will not be observed
+ // and can be dropped).
+ bool keepSet = newIndex != RemovedField;
+ if (keepSet) {
+ auto immIter = parent.canBecomeImmutable.find(heapType);
+ if (immIter != parent.canBecomeImmutable.end()) {
+ auto& immutableVec = immIter->second;
+ if (curr->index < immutableVec.size() && immutableVec[curr->index]) {
+ keepSet = false;
+ }
+ }
+ }
+ if (keepSet) {
// Map to the new index.
curr->index = newIndex;
} else {
@@ -364,8 +396,16 @@
}
auto newIndex = getNewIndex(curr->ref->type.getHeapType(), curr->index);
- // We must not remove a field that is read from.
- assert(newIndex != RemovedField);
+ if (newIndex == RemovedField) {
+ // This is a read of the field, but the field has no escaping reads so
+ // we removed it. That means that the field is read only to be written
+ // back into the field, which means we can ignore the read and
+ // replace it with something.
+ Builder builder(*getModule());
+ // TODO: traps
+ replaceCurrent(builder.replaceWithIdenticalType(curr));
+ }
+
curr->index = newIndex;
}
diff --git a/src/passes/TypeRefining.cpp b/src/passes/TypeRefining.cpp
index 74b23bb..b84fe09 100644
--- a/src/passes/TypeRefining.cpp
+++ b/src/passes/TypeRefining.cpp
@@ -50,7 +50,7 @@
: StructUtils::StructScanner<FieldInfo, FieldInfoScanner>(
functionNewInfos, functionSetGetInfos) {}
- void noteExpression(Expression* expr,
+ void noteWrite(Expression* expr,
HeapType type,
Index index,
FieldInfo& info) {
diff --git a/test/lit/passes/gto-removals.wast b/test/lit/passes/gto-removals.wast
index fc5ed90..da84dda 100644
--- a/test/lit/passes/gto-removals.wast
+++ b/test/lit/passes/gto-removals.wast
@@ -115,6 +115,66 @@
)
(module
+ ;; A copy does *not* keep a field from being removed, since the value does not
+ ;; escape and is therefore not observable.
+
+ ;; CHECK: (type $ref|$struct|_=>_none (func_subtype (param (ref $struct)) func))
+
+ ;; CHECK: (type $struct (struct_subtype data))
+ (type $struct (struct_subtype (field (mut funcref)) data))
+
+ ;; CHECK: (func $func (type $ref|$struct|_=>_none) (param $x (ref $struct))
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (local.get $x)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (ref.null func)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ (func $func (param $x (ref $struct))
+ (struct.set $struct 0
+ (local.get $x)
+ (struct.get $struct 0
+ (local.get $x)
+ )
+ )
+ )
+)
+
+(module
+ ;; A read-only-to-write operation like an increment also does not keep a field
+ ;; from being removed.
+
+ ;; CHECK: (type $ref|$struct|_=>_none (func_subtype (param (ref $struct)) func))
+
+ ;; CHECK: (type $struct (struct_subtype data))
+ (type $struct (struct_subtype (field (mut i32)) data))
+
+ ;; CHECK: (func $func (type $ref|$struct|_=>_none) (param $x (ref $struct))
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (local.get $x)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (i32.add
+ ;; CHECK-NEXT: (i32.const 0)
+ ;; CHECK-NEXT: (i32.const 1)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ (func $func (param $x (ref $struct))
+ (struct.set $struct 0
+ (local.get $x)
+ (i32.add
+ (struct.get $struct 0
+ (local.get $x)
+ )
+ (i32.const 1)
+ )
+ )
+ )
+)
+
+(module
;; Different struct types with different situations: some fields are read,
;; some written, and some both. (Note that this also tests the interaction
;; of removing with the immutability inference that --gto does.)
