Skip to content
Merged
Show file tree
Hide file tree
Changes from 21 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
23 changes: 20 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 @@ -2829,6 +2837,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 @@ -3188,7 +3199,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 +3247,12 @@ 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.

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 This has run against the fuzzer for a day. Did you want to review the above changes or is this good to land?

Copy link
Member

Choose a reason for hiding this comment

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

lgtm with the suggested TODO

// If I do this I get:
// Flower::updateContents(LocationIndex, PossibleContents): Assertion `!contents.isMany()' failed.
// valueContents = PossibleContents::fromType(value->type);
valueContents = PossibleContents::fromType(Type::i32);
}

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
13 changes: 11 additions & 2 deletions src/wasm-binary.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ enum {
MaxLEB32Bytes = 5,
};

enum class BackingType {
Memory,
Array,
};

template<typename T, typename MiniT> struct LEB {
static_assert(sizeof(MiniT) == 1, "MiniT must be a byte");

Expand Down Expand Up @@ -359,6 +364,7 @@ enum BrOnCastFlag {

constexpr uint32_t ExactImport = 1 << 5;

constexpr uint32_t HasBackingArrayMask = 1 << 4;
constexpr uint32_t HasMemoryOrderMask = 1 << 5;
constexpr uint32_t HasMemoryIndexMask = 1 << 6;

Expand Down Expand Up @@ -463,6 +469,7 @@ extern const char* BulkMemoryOptFeature;
extern const char* CallIndirectOverlongFeature;
extern const char* CustomDescriptorsFeature;
extern const char* RelaxedAtomicsFeature;
extern const char* MultibyteFeature;
extern const char* CustomPageSizesFeature;

enum Subsection {
Expand Down Expand Up @@ -1709,6 +1716,8 @@ class WasmBinaryReader {

void readExports();

Result<> readStore(unsigned bytes, Type type);

// The strings in the strings section (which are referred to by StringConst).
std::vector<Name> strings;
void readStrings();
Expand Down Expand Up @@ -1756,11 +1765,11 @@ class WasmBinaryReader {
void readJSCalledHints(size_t payloadLen);
void readIdempotentHints(size_t payloadLen);

std::tuple<Address, Address, Index, MemoryOrder>
std::tuple<Address, Address, Index, MemoryOrder, BackingType>
readMemoryAccess(bool isAtomic, bool isRMW);
std::tuple<Name, Address, Address, MemoryOrder> getAtomicMemarg();
std::tuple<Name, Address, Address, MemoryOrder> getRMWMemarg();
std::tuple<Name, Address, Address> getMemarg();
std::tuple<Name, Address, Address, BackingType> getMemarg();
MemoryOrder getMemoryOrder(bool isRMW = false);

[[noreturn]] void throwError(std::string text) {
Expand Down
Loading
Loading