Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/fuzz_opt.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
# V8 does not support shared memories when running with
# shared-everything enabled, so do not fuzz shared-everything
# for now. The remaining features are not yet implemented in v8.
DISALLOWED_FEATURES_IN_V8 = ['shared-everything', 'strings', 'stack-switching', 'relaxed-atomics']
DISALLOWED_FEATURES_IN_V8 = ['shared-everything', 'strings', 'stack-switching', 'relaxed-atomics', 'multibyte']


# utilities
Expand Down
3 changes: 3 additions & 0 deletions src/binaryen-c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,9 @@ BinaryenFeatures BinaryenFeatureCallIndirectOverlong(void) {
BinaryenFeatures BinaryenFeatureRelaxedAtomics(void) {
return static_cast<BinaryenFeatures>(FeatureSet::RelaxedAtomics);
}
BinaryenFeatures BinaryenFeatureMultibyte(void) {
return static_cast<BinaryenFeatures>(FeatureSet::Multibyte);
}
BinaryenFeatures BinaryenFeatureCustomPageSizes(void) {
return static_cast<BinaryenFeatures>(FeatureSet::CustomPageSizes);
}
Expand Down
1 change: 1 addition & 0 deletions src/binaryen-c.h
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ BINARYEN_API BinaryenFeatures BinaryenFeatureFP16(void);
BINARYEN_API BinaryenFeatures BinaryenFeatureBulkMemoryOpt(void);
BINARYEN_API BinaryenFeatures BinaryenFeatureCallIndirectOverlong(void);
BINARYEN_API BinaryenFeatures BinaryenFeatureRelaxedAtomics(void);
BINARYEN_API BinaryenFeatures BinaryenFeatureMultibyte(void);
BINARYEN_API BinaryenFeatures BinaryenFeatureCustomPageSizes(void);
BINARYEN_API BinaryenFeatures BinaryenFeatureAll(void);

Expand Down
1 change: 1 addition & 0 deletions src/interpreter/interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ struct ExpressionInterpreter : OverriddenVisitor<ExpressionInterpreter, Flow> {
Flow visitArrayNewFixed(ArrayNewFixed* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitArrayGet(ArrayGet* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitArraySet(ArraySet* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitArrayStore(ArrayStore* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitArrayLen(ArrayLen* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitArrayCopy(ArrayCopy* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitArrayFill(ArrayFill* curr) { WASM_UNREACHABLE("TODO"); }
Expand Down
1 change: 1 addition & 0 deletions src/ir/ReFinalize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ void ReFinalize::visitArrayNewElem(ArrayNewElem* curr) { curr->finalize(); }
void ReFinalize::visitArrayNewFixed(ArrayNewFixed* curr) { curr->finalize(); }
void ReFinalize::visitArrayGet(ArrayGet* curr) { curr->finalize(); }
void ReFinalize::visitArraySet(ArraySet* curr) { curr->finalize(); }
void ReFinalize::visitArrayStore(ArrayStore* curr) { curr->finalize(); }
void ReFinalize::visitArrayLen(ArrayLen* curr) { curr->finalize(); }
void ReFinalize::visitArrayCopy(ArrayCopy* curr) { curr->finalize(); }
void ReFinalize::visitArrayFill(ArrayFill* curr) { curr->finalize(); }
Expand Down
20 changes: 20 additions & 0 deletions src/ir/child-typer.h
Original file line number Diff line number Diff line change
Expand Up @@ -1096,6 +1096,26 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
note(&curr->value, type);
}

void visitArrayStore(ArrayStore* curr,
std::optional<HeapType> ht = std::nullopt,
std::optional<Type> valueType = std::nullopt) {
if (!ht) {
if (!curr->ref->type.isRef()) {
self().noteUnknown();
return;
}
ht = curr->ref->type.getHeapType();
}
auto actualValueType = valueType ? *valueType : curr->value->type;
if (actualValueType == Type::unreachable) {
self().noteUnknown();
return;
}
note(&curr->ref, Type(*ht, Nullable));
note(&curr->index, Type::i32);
note(&curr->value, actualValueType);
}

void visitArrayLen(ArrayLen* curr) {
note(&curr->ref, Type(HeapType::array, Nullable));
}
Expand Down
4 changes: 4 additions & 0 deletions src/ir/cost.h
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,10 @@ struct CostAnalyzer : public OverriddenVisitor<CostAnalyzer, CostType> {
return 2 + nullCheckCost(curr->ref) + visit(curr->ref) +
visit(curr->index) + visit(curr->value);
}
CostType visitArrayStore(ArrayStore* curr) {
return 2 + nullCheckCost(curr->ref) + visit(curr->ref) +
visit(curr->index) + visit(curr->value);
}
CostType visitArrayLen(ArrayLen* curr) {
return 1 + nullCheckCost(curr->ref) + visit(curr->ref);
}
Expand Down
9 changes: 9 additions & 0 deletions src/ir/effects.h
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,15 @@ class EffectAnalyzer {
parent.implicitTrap = true;
writesArray(curr->ref->type.getHeapType(), curr->order);
}
void visitArrayStore(ArrayStore* curr) {
if (curr->ref->type.isNull()) {
parent.trap = true;
return;
}
parent.writesArray = true;
// traps when the arg is null or the index out of bounds
parent.implicitTrap = true;
}
void visitArrayLen(ArrayLen* curr) {
trapOnNull(curr->ref);
// No need to model this as reading the array since the length cannot be
Expand Down
30 changes: 27 additions & 3 deletions src/ir/possible-contents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1102,6 +1102,13 @@ struct InfoCollector
addChildParentLink(curr->ref, curr);
addChildParentLink(curr->value, curr);
}
void visitArrayStore(ArrayStore* curr) {
if (curr->ref->type == Type::unreachable) {
return;
}
addChildParentLink(curr->ref, curr);
addChildParentLink(curr->value, curr);
}

void visitArrayLen(ArrayLen* curr) {
// TODO: optimize when possible (perhaps we can infer a Literal for the
Expand Down Expand Up @@ -1742,6 +1749,7 @@ void TNHOracle::scan(Function* func,
}
void visitArrayGet(ArrayGet* curr) { notePossibleTrap(curr->ref); }
void visitArraySet(ArraySet* curr) { notePossibleTrap(curr->ref); }
void visitArrayStore(ArrayStore* curr) { notePossibleTrap(curr->ref); }
void visitArrayLen(ArrayLen* curr) { notePossibleTrap(curr->ref); }
void visitArrayCopy(ArrayCopy* curr) {
notePossibleTrap(curr->srcRef);
Expand Down Expand Up @@ -2239,7 +2247,7 @@ struct Flower {
Expression* read);

// Similar to readFromData, but does a write for a struct.set or array.set.
void writeToData(Expression* ref, Expression* value, Index fieldIndex);
void writeToData(Expression* ref, Expression* value, Index fieldIndex, bool multibyte = false);

// We will need subtypes during the flow, so compute them once ahead of time.
std::unique_ptr<SubTypes> subTypes;
Expand Down Expand Up @@ -2576,7 +2584,7 @@ Flower::Flower(Module& wasm, const PassOptions& options)
for (const auto& [location, value] : roots) {
#if defined(POSSIBLE_CONTENTS_DEBUG) && POSSIBLE_CONTENTS_DEBUG >= 2
std::cout << " init root\n";
dump(location);
dump(getLocation(location));
value.dump(std::cout, &wasm);
std::cout << '\n';
#endif
Expand Down Expand Up @@ -2743,6 +2751,11 @@ bool Flower::updateContents(LocationIndex locationIndex,
} else if (auto* globalLoc = std::get_if<GlobalLocation>(&location)) {
filterGlobalContents(contents, *globalLoc);
filtered = true;
} else if (auto* dataLoc = std::get_if<DataLocation>(&location)) {
// Multibyte array stores with differing widths can merge to Many, so
// filter again afterwards to fall back to the declared type limit.
filterDataContents(contents, *dataLoc);
filtered = true;
}

// Check if anything changed after filtering, if we did so.
Expand Down Expand Up @@ -2829,6 +2842,9 @@ void Flower::flowAfterUpdate(LocationIndex locationIndex) {
} else if (auto* set = parent->dynCast<ArraySet>()) {
assert(set->ref == child || set->value == child);
writeToData(set->ref, set->value, 0);
} else if (auto* store = parent->dynCast<ArrayStore>()) {
assert(store->ref == child || store->value == child);
writeToData(store->ref, store->value, 0, /*multibyte*/ true);
} else if (auto* get = parent->dynCast<RefGetDesc>()) {
// Similar to struct.get.
assert(get->ref == child);
Expand Down Expand Up @@ -3006,6 +3022,11 @@ void Flower::filterDataContents(PossibleContents& contents,
contents = PossibleContents::none();
return;
}
if (contents.isMany()) {
// An unknown state (e.g. from combining writes of different types like in
// multibyte array stores) translates into the most generic bounded type.
contents = PossibleContents::fromType(field->type);
}

if (field->isPacked()) {
// We must handle packed fields carefully.
Expand Down Expand Up @@ -3188,7 +3209,7 @@ void Flower::readFromData(Type declaredType,
connectDuringFlow(coneReadLocation, ExpressionLocation{read, 0});
}

void Flower::writeToData(Expression* ref, Expression* value, Index fieldIndex) {
void Flower::writeToData(Expression* ref, Expression* value, Index fieldIndex, bool multibyte) {
#if defined(POSSIBLE_CONTENTS_DEBUG) && POSSIBLE_CONTENTS_DEBUG >= 2
std::cout << " add special writes\n";
#endif
Expand Down Expand Up @@ -3236,6 +3257,9 @@ void Flower::writeToData(Expression* ref, Expression* value, Index fieldIndex) {
// As in readFromData, normalize to the proper cone.
auto cone = refContents.getCone();
auto normalizedDepth = getNormalizedConeDepth(cone.type, cone.depth);
if (multibyte) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kripken Here's where I'm having issues.

(module
 (type $0 (func))
 (type $1 (array (mut i8)))
 (global $global$0 (ref $1) (array.new_default $1
  (i32.const 4)
 ))
 (func $0
  (f64.store (type $1)
   (global.get $global$0)
   (i32.const 1)
   (f64.const 2)
  )
 )
)

with
wasm-opt a.wasm -o b.wasm --discard-global-effects --roundtrip --signature-refining --gufa-cast-all --shrink-level=1 --closed-world --dce --closed-world -all

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am guessing that filterExpressionContents applies the type of the expression in the wasm, and here it writes a different type, so the filtering ends up making things worse.

Updating filterExpressionContents should fix this. I assume the issue is on ArrayGet. When such a get sees data of another type in a numeric array, rather than combining them to get Many, we can apply PossibleContents::fromType(value->type). That is, an unknown value in array.get is still going to be of the type that particular get reads.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright here's what I came up with in c148eaf . I'll let it cook on the fuzzer for a bit.

valueContents = PossibleContents::fromType(value->type);
}

subTypes->iterSubTypes(
cone.type.getHeapType(), normalizedDepth, [&](HeapType type, Index depth) {
Expand Down
1 change: 1 addition & 0 deletions src/ir/subtype-exprs.h
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> {
auto array = curr->ref->type.getHeapType().getArray();
self()->noteSubtype(curr->value, array.element.type);
}
void visitArrayStore(ArrayStore* curr) {}
void visitArrayLen(ArrayLen* curr) {}
void visitArrayCopy(ArrayCopy* curr) {
if (!curr->srcRef->type.isArray() || !curr->destRef->type.isArray()) {
Expand Down
13 changes: 13 additions & 0 deletions src/parser/contexts.h
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,11 @@ struct NullInstrParserCtx {
MemoryOrder) {
return Ok{};
}
template<typename HeapTypeT>
Result<>
makeArrayStore(Index, const std::vector<Annotation>&, Type, int, HeapTypeT) {
return Ok{};
}
Result<> makeAtomicRMW(Index,
const std::vector<Annotation>&,
AtomicRMWOp,
Expand Down Expand Up @@ -2326,6 +2331,14 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx>, AnnotationParserCtx {
pos, irBuilder.makeStore(bytes, memarg.offset, memarg.align, type, *m));
}

Result<> makeArrayStore(Index pos,
const std::vector<Annotation>& annotations,
Type type,
int bytes,
HeapTypeT arrayType) {
return withLoc(pos, irBuilder.makeArrayStore(arrayType, bytes, type));
}

Result<> makeAtomicRMW(Index pos,
const std::vector<Annotation>& annotations,
AtomicRMWOp op,
Expand Down
10 changes: 10 additions & 0 deletions src/parser/parsers.h
Original file line number Diff line number Diff line change
Expand Up @@ -1818,6 +1818,16 @@ Result<> makeStore(Ctx& ctx,
Type type,
int bytes,
bool isAtomic) {
if (ctx.in.takeSExprStart("type"sv)) {
auto arrayType = typeidx(ctx);
CHECK_ERR(arrayType);

if (!ctx.in.takeRParen()) {
return ctx.in.err("expected end of type use");
}

return ctx.makeArrayStore(pos, annotations, type, bytes, *arrayType);
}
auto mem = maybeMemidx(ctx);
CHECK_ERR(mem);

Expand Down
69 changes: 41 additions & 28 deletions src/passes/Print.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,33 @@ struct PrintExpressionContents
return parent.printBlockType(sig);
}

void printMemoryPostfix(uint8_t bytes, Type type) {
switch (bytes) {
case 1:
o << '8';
break;
case 2:
if (type == Type::f32) {
o << "_f16";
} else {
o << "16";
}
break;
case 4:
o << "32";
break;
default:
abort();
}
}

std::ostream& printStorePostfix(uint8_t bytes, Type valueType) {
if (bytes < 4 || (valueType == Type::i64 && bytes < 8)) {
printMemoryPostfix(bytes, valueType);
}
return o;
}

void visitBlock(Block* curr) {
printMedium(o, "block");
if (curr->name.is()) {
Expand Down Expand Up @@ -556,19 +583,7 @@ struct PrintExpressionContents
o << ".load";
if (curr->type != Type::unreachable &&
curr->bytes < curr->type.getByteSize()) {
if (curr->bytes == 1) {
o << '8';
} else if (curr->bytes == 2) {
if (curr->type == Type::f32) {
o << "_f16";
} else {
o << "16";
}
} else if (curr->bytes == 4) {
o << "32";
} else {
abort();
}
printMemoryPostfix(curr->bytes, curr->type);
if (curr->type != Type::f32) {
o << (curr->signed_ ? "_s" : "_u");
}
Expand All @@ -589,21 +604,7 @@ struct PrintExpressionContents
o << ".atomic";
}
o << ".store";
if (curr->bytes < 4 || (curr->valueType == Type::i64 && curr->bytes < 8)) {
if (curr->bytes == 1) {
o << '8';
} else if (curr->bytes == 2) {
if (curr->valueType == Type::f32) {
o << "_f16";
} else {
o << "16";
}
} else if (curr->bytes == 4) {
o << "32";
} else {
abort();
}
}
printStorePostfix(curr->bytes, curr->valueType);
restoreNormalColor(o);
printMemoryName(curr->memory, o, wasm);
printMemoryOrder(curr->order);
Expand Down Expand Up @@ -2477,6 +2478,18 @@ struct PrintExpressionContents
o << ' ';
printHeapTypeName(curr->ref->type.getHeapType());
}
void visitArrayStore(ArrayStore* curr) {
prepareColor(o) << forceConcrete(curr->value->type);
o << ".store";
printStorePostfix(curr->bytes, curr->value->type);
o << " ";
restoreNormalColor(o);

o << '(';
printMinor(o, "type ");
printHeapTypeName(curr->ref->type.getHeapType());
o << ')';
}
void visitArrayLen(ArrayLen* curr) { printMedium(o, "array.len"); }
void visitArrayCopy(ArrayCopy* curr) {
printMedium(o, "array.copy ");
Expand Down
2 changes: 2 additions & 0 deletions src/passes/TypeGeneralizing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,8 @@ struct TransferFn : OverriddenVisitor<TransferFn> {
}
}

void visitArrayStore(ArrayStore* curr) { WASM_UNREACHABLE("TODO"); }

void visitArrayLen(ArrayLen* curr) {
// The input must be an array.
push(Type(HeapType::array, Nullable));
Expand Down
1 change: 1 addition & 0 deletions src/tools/tool-options.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ struct ToolOptions : public Options {
.addFeature(FeatureSet::FP16, "float 16 operations")
.addFeature(FeatureSet::CustomDescriptors,
"custom descriptors (RTTs) and exact references")
.addFeature(FeatureSet::Multibyte, "multibyte array loads and stores")
.addFeature(FeatureSet::RelaxedAtomics,
"acquire/release atomic memory operations")
.addFeature(FeatureSet::CustomPageSizes, "custom page sizes")
Expand Down
Loading
Loading