Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions scripts/gen-s-parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@
("memory.atomic.wait64", "makeAtomicWait(Type::i64)"),
("atomic.fence", "makeAtomicFence()"),
("pause", "makePause()"),
("publish", "makePublish()"),
("i32.atomic.load8_u", "makeLoad(Type::i32, /*signed=*/false, 1, /*isAtomic=*/true)"),
("i32.atomic.load16_u", "makeLoad(Type::i32, /*signed=*/false, 2, /*isAtomic=*/true)"),
("i32.atomic.load", "makeLoad(Type::i32, /*signed=*/false, 4, /*isAtomic=*/true)"),
Expand Down
1 change: 1 addition & 0 deletions scripts/test/fuzzing.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@
# Not fully implemented.
'waitqueue.wast',
'gufa-waitqueue.wast',
'publish.wast',
# TODO: fix handling of the non-utf8 names here
'name-high-bytes.wast',
# JS interop testcases have complex js-wasm interactions
Expand Down
18 changes: 18 additions & 0 deletions src/binaryen-c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1909,6 +1909,10 @@ BinaryenExpressionRef BinaryenWaitqueueNotify(BinaryenModuleRef module,
return Builder(*(Module*)module)
.makeWaitqueueNotify((Expression*)waitqueue, (Expression*)count);
}
BinaryenExpressionRef BinaryenPublish(BinaryenModuleRef module,
BinaryenExpressionRef ref) {
return Builder(*(Module*)module).makePublish((Expression*)ref);
}
BinaryenExpressionRef BinaryenArrayNew(BinaryenModuleRef module,
BinaryenHeapType type,
BinaryenExpressionRef size,
Expand Down Expand Up @@ -4629,6 +4633,20 @@ void BinaryenWaitqueueNotifySetCount(BinaryenExpressionRef expr,
static_cast<WaitqueueNotify*>(expression)->count = (Expression*)countExpr;
}

// Publish
BinaryenExpressionRef BinaryenPublishGetRef(BinaryenExpressionRef expr) {
auto* expression = (Expression*)expr;
assert(expression->is<Publish>());
return static_cast<Publish*>(expression)->ref;
}
void BinaryenPublishSetRef(BinaryenExpressionRef expr,
BinaryenExpressionRef refExpr) {
auto* expression = (Expression*)expr;
assert(expression->is<Publish>());
assert(refExpr);
static_cast<Publish*>(expression)->ref = (Expression*)refExpr;
}

// ArrayNew
BinaryenExpressionRef BinaryenArrayNewGetInit(BinaryenExpressionRef expr) {
auto* expression = (Expression*)expr;
Expand Down
9 changes: 9 additions & 0 deletions src/binaryen-c.h
Original file line number Diff line number Diff line change
Expand Up @@ -1110,6 +1110,8 @@ BINARYEN_API BinaryenExpressionRef
BinaryenWaitqueueNotify(BinaryenModuleRef module,
BinaryenExpressionRef waitqueue,
BinaryenExpressionRef count);
BINARYEN_API BinaryenExpressionRef BinaryenPublish(BinaryenModuleRef module,
BinaryenExpressionRef ref);
BINARYEN_API BinaryenExpressionRef BinaryenArrayNew(BinaryenModuleRef module,
BinaryenHeapType type,
BinaryenExpressionRef size,
Expand Down Expand Up @@ -2618,6 +2620,13 @@ BINARYEN_API void
BinaryenWaitqueueNotifySetCount(BinaryenExpressionRef expr,
BinaryenExpressionRef countExpr);

// Publish

BINARYEN_API BinaryenExpressionRef
BinaryenPublishGetRef(BinaryenExpressionRef expr);
BINARYEN_API void BinaryenPublishSetRef(BinaryenExpressionRef expr,
BinaryenExpressionRef refExpr);

// ArrayNew

BINARYEN_API BinaryenExpressionRef
Expand Down
6 changes: 6 additions & 0 deletions src/gen-s-parser.inc
Original file line number Diff line number Diff line change
Expand Up @@ -4948,6 +4948,12 @@ switch (buf[0]) {
return Ok{};
}
goto parse_error;
case 'u':
if (op == "publish"sv) {
CHECK_ERR(makePublish(ctx, pos, annotations));
return Ok{};
}
goto parse_error;
default: goto parse_error;
}
}
Expand Down
1 change: 1 addition & 0 deletions src/interpreter/interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ struct ExpressionInterpreter : OverriddenVisitor<ExpressionInterpreter, Flow> {
Flow visitStructWait(StructWait* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitWaitqueueNew(WaitqueueNew* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitWaitqueueNotify(WaitqueueNotify* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitPublish(Publish* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitArrayNew(ArrayNew* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitArrayNewData(ArrayNewData* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitArrayNewElem(ArrayNewElem* 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 @@ -170,6 +170,7 @@ void ReFinalize::visitWaitqueueNew(WaitqueueNew* curr) { curr->finalize(); }
void ReFinalize::visitWaitqueueNotify(WaitqueueNotify* curr) {
curr->finalize();
}
void ReFinalize::visitPublish(Publish* curr) { curr->finalize(); }
void ReFinalize::visitArrayNew(ArrayNew* curr) { curr->finalize(); }
void ReFinalize::visitArrayNewData(ArrayNewData* curr) { curr->finalize(); }
void ReFinalize::visitArrayNewElem(ArrayNewElem* curr) { curr->finalize(); }
Expand Down
5 changes: 5 additions & 0 deletions src/ir/child-typer.h
Original file line number Diff line number Diff line change
Expand Up @@ -1056,6 +1056,11 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
note(&curr->count, Type(Type::BasicType::i32));
}

void visitPublish(Publish* curr) {
// Polymorphic over heap types.
note(&curr->ref, VarRef{Nullable, VarHeapType{0u}});
}

void visitArrayNew(ArrayNew* curr) {
if (!curr->isWithDefault()) {
if (!curr->type.isRef()) {
Expand Down
1 change: 1 addition & 0 deletions src/ir/cost.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ struct CostAnalyzer : public OverriddenVisitor<CostAnalyzer, CostType> {
return AtomicCost + nullCheckCost(curr->waitqueue) +
visit(curr->waitqueue) + visit(curr->count);
}
CostType visitPublish(Publish* curr) { return AtomicCost + visit(curr->ref); }
CostType visitAtomicNotify(AtomicNotify* curr) {
return AtomicCost + visit(curr->ptr) + visit(curr->notifyCount);
}
Expand Down
49 changes: 49 additions & 0 deletions src/ir/effects.h
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,55 @@ class EffectAnalyzer {
parent.writesSharedStruct = true;
parent.readOrder = parent.writeOrder = MemoryOrder::SeqCst;
}
void visitPublish(Publish* curr) {
// Publish is a no-op on anything besides shared structs and arrays.
if (!curr->ref->type.isRef()) {
return;
}
auto heapType = curr->ref->type.getHeapType();
if (!heapType.isShared()) {
return;
}
if (!heapType.isStruct() && !heapType.isMaybeShared(HeapType::struct_) &&
!heapType.isArray() && !heapType.isMaybeShared(HeapType::array) &&
!heapType.isMaybeShared(HeapType::eq) &&
!heapType.isMaybeShared(HeapType::any)) {
return;
}
// TODO: Modeling `publish` as an arbitrary call is not sufficient. While
// it conservatively prevents writes to global state (shared or unshared
// structs/arrays, globals, memory, tables) from reordering across
// `publish`, it has two major shortcomings:
//
// 1. It is overly conservative for reads and independent writes: it
// prevents reads of all kinds from reordering around `publish`, even
// though reads of all memory orders should be allowed to reorder
// freely. It also blocks unrelated writes from reordering.
// 2. It does NOT prevent writes of the published object to local
// variables from moving before `publish`, because calls do not
// conflict with local accesses. For example, in:
// (local.set $p (publish (local.get $s)))
// (local.set $x (local.get $s))
// `SimplifyLocals` could sink `publish` past `local.set $x`, moving
// the write of `$s` to `$x` before `publish`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wait, I'm not following. The write of s to x is fine, isn't it? Isn't the danger only writes to the heap object a local refers to?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Possibly. I was thinking about this this morning and I'm actually not sure how this should work with respect to aliasing in locals. It's a question for Conrad, probably.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, no, thinking about this more, writes to locals are definitely not a problem. The chain of address dependencies, data dependencies, and happens-before edges leading up to a read on another thread must start with a write of the published object that happens after the publish, not with the publish itself. So it's ok if that write after the publish has no data dependency on the publish, which means it's fine to move local.set of the published object before the publish.

I'll update the comment.

//
// A potential design to model `publish` properly would track new data in
// `EffectAnalyzer`:
//
// - `publishedLocals`: A set of local indices published by `publish`
// (e.g. from `curr->ref`). Any expression that writes to a local or
// global state while reading a published local would conflict with
// `publish`, preventing writes of the published object to unshared
// locations or locals from hoisting before `publish`. Pure reads would
// not be marked as writing and could reorder freely.
// - `publishedHeapTypes` and `writtenValueHeapTypes`: To safely handle
// aliasing across locals and non-local expressions without external
// analysis, `EffectAnalyzer` could track the heap types of published
// references and compare them against the types of values being
// written in `LocalSet`, `GlobalSet`, `TableSet`, `StructSet`, and
// `ArraySet`.
parent.calls = true;
}
void visitArrayNew(ArrayNew* curr) {}
void visitArrayNewData(ArrayNewData* curr) {
// Traps on out of bounds access to segments or access to dropped
Expand Down
1 change: 1 addition & 0 deletions src/ir/possible-contents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,7 @@ struct InfoCollector
void visitStructWait(StructWait* curr) { addRoot(curr); }
void visitWaitqueueNew(WaitqueueNew* curr) { addRoot(curr); }
void visitWaitqueueNotify(WaitqueueNotify* curr) { addRoot(curr); }
void visitPublish(Publish* curr) { receiveChildValue(curr->ref, curr); }
// Array operations access the array's location, parallel to how structs work.
void visitArrayGet(ArrayGet* curr) {
if (!isRelevant(curr->ref)) {
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 @@ -397,6 +397,7 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> {
self()->noteSubtype(curr->waitqueue,
Type(HeapTypes::sharedWaitqueue, Nullable));
}
void visitPublish(Publish* curr) {}
void visitArrayNew(ArrayNew* curr) {
if (!curr->type.isArray() || curr->isWithDefault()) {
return;
Expand Down
14 changes: 14 additions & 0 deletions src/js/binaryen.js-post.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ function initializeConstants() {
'StructWait',
'WaitqueueNew',
'WaitqueueNotify',
'Publish',
'ArrayNew',
'ArrayNewFixed',
'ArrayNewData',
Expand Down Expand Up @@ -2609,6 +2610,10 @@ function wrapModule(module, self = {}) {
}
};

self['publish'] = function(ref) {
return Module['_BinaryenPublish'](module, ref);
};

self['array'] = {
'new'(type, size, init) {
return Module['_BinaryenArrayNew'](module, type, size, init);
Expand Down Expand Up @@ -4994,6 +4999,15 @@ Module['WaitqueueNotify'] = makeExpressionWrapper(Module['_BinaryenWaitqueueNoti
}
});

Module['Publish'] = makeExpressionWrapper(Module['_BinaryenPublishId'](), {
'getRef'(expr) {
return Module['_BinaryenPublishGetRef'](expr);
},
'setRef'(expr, refExpr) {
Module['_BinaryenPublishSetRef'](expr, refExpr);
}
});

Module['ArrayNew'] = makeExpressionWrapper(Module['_BinaryenArrayNewId'](), {
'getInit'(expr) {
return Module['_BinaryenArrayNewGetInit'](expr);
Expand Down
5 changes: 5 additions & 0 deletions src/parser/contexts.h
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,7 @@ struct NullInstrParserCtx {
return Ok{};
}
Result<> makePause(Index, const std::vector<Annotation>&) { return Ok{}; }
Result<> makePublish(Index, const std::vector<Annotation>&) { return Ok{}; }
Result<> makeSIMDExtract(Index,
const std::vector<Annotation>&,
SIMDExtractOp,
Expand Down Expand Up @@ -2506,6 +2507,10 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx>, AnnotationParserCtx {
return withLoc(pos, irBuilder.makePause());
}

Result<> makePublish(Index pos, const std::vector<Annotation>& annotations) {
return withLoc(pos, irBuilder.makePublish());
}

Result<> makeSIMDExtract(Index pos,
const std::vector<Annotation>& annotations,
SIMDExtractOp op,
Expand Down
8 changes: 8 additions & 0 deletions src/parser/parsers.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ makeAtomicFence(Ctx&, Index, const std::vector<Annotation>&, MemoryOrder);
template<typename Ctx>
Result<> makePause(Ctx&, Index, const std::vector<Annotation>&);
template<typename Ctx>
Result<> makePublish(Ctx&, Index, const std::vector<Annotation>&);
template<typename Ctx>
Result<> makeSIMDExtract(
Ctx&, Index, const std::vector<Annotation>&, SIMDExtractOp op, size_t lanes);
template<typename Ctx>
Expand Down Expand Up @@ -1995,6 +1997,12 @@ makePause(Ctx& ctx, Index pos, const std::vector<Annotation>& annotations) {
return ctx.makePause(pos, annotations);
}

template<typename Ctx>
Result<>
makePublish(Ctx& ctx, Index pos, const std::vector<Annotation>& annotations) {
return ctx.makePublish(pos, annotations);
}

template<typename Ctx>
Result<> makeSIMDExtract(Ctx& ctx,
Index pos,
Expand Down
1 change: 1 addition & 0 deletions src/passes/Print.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2408,6 +2408,7 @@ struct PrintExpressionContents
void visitWaitqueueNotify(WaitqueueNotify* curr) {
printMedium(o, "waitqueue.notify");
}
void visitPublish(Publish* curr) { printMedium(o, "publish"); }
void visitArrayNew(ArrayNew* curr) {
printMedium(o, "array.new");
if (curr->isWithDefault()) {
Expand Down
2 changes: 2 additions & 0 deletions src/passes/TypeGeneralizing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,8 @@ struct TransferFn : OverriddenVisitor<TransferFn> {

void visitWaitqueueNotify(WaitqueueNotify* curr) { WASM_UNREACHABLE("TODO"); }

void visitPublish(Publish* curr) { WASM_UNREACHABLE("TODO"); }

void visitArrayNew(ArrayNew* curr) {
// We cannot yet generalize allocations. Push a requirement for the
// reference type needed to initialize the array, if any.
Expand Down
1 change: 1 addition & 0 deletions src/wasm-binary.h
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,7 @@ enum ASTNodes {
StructWait = 0x05,
WaitqueueNotify = 0x06,
WaitqueueNew = 0x07,
Publish = 0x0f,

I32AtomicLoad = 0x10,
I64AtomicLoad = 0x11,
Expand Down
7 changes: 7 additions & 0 deletions src/wasm-builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -1474,6 +1474,13 @@ class Builder {
return ret;
}

Publish* makePublish(Expression* ref) {
auto* ret = wasm.allocator.alloc<Publish>();
ret->ref = ref;
ret->finalize();
return ret;
}

// Additional helpers

Drop* makeDrop(Expression* value) {
Expand Down
4 changes: 4 additions & 0 deletions src/wasm-delegations-fields.def
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,10 @@ DELEGATE_FIELD_CHILD(WaitqueueNotify, count)
DELEGATE_FIELD_CHILD(WaitqueueNotify, waitqueue)
DELEGATE_FIELD_CASE_END(WaitqueueNotify)

DELEGATE_FIELD_CASE_START(Publish)
DELEGATE_FIELD_CHILD(Publish, ref)
DELEGATE_FIELD_CASE_END(Publish)

DELEGATE_FIELD_CASE_START(WideIntAddSub)
DELEGATE_FIELD_INT(WideIntAddSub, op)
DELEGATE_FIELD_CHILD(WideIntAddSub, rightHigh)
Expand Down
1 change: 1 addition & 0 deletions src/wasm-delegations.def
Original file line number Diff line number Diff line change
Expand Up @@ -122,5 +122,6 @@ DELEGATE(WideIntAddSub);
DELEGATE(WideIntMul);
DELEGATE(WaitqueueNew);
DELEGATE(WaitqueueNotify);
DELEGATE(Publish);

#undef DELEGATE
6 changes: 6 additions & 0 deletions src/wasm-interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -2370,6 +2370,11 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
return Literal(int32_t{0}); // none woken up
}

Flow visitPublish(Publish* curr) {
VISIT(ref, curr->ref)
return ref;
}

// Arbitrary deterministic limit on size. If we need to allocate a Literals
// vector that takes around 1-2GB of memory then we are likely to hit memory
// limits on 32-bit machines, and in particular on wasm32 VMs that do not
Expand Down Expand Up @@ -3111,6 +3116,7 @@ class ConstantExpressionRunner : public ExpressionRunner<SubType> {
Flow visitWaitqueueNotify(WaitqueueNotify* curr) {
return Flow(NONCONSTANT_FLOW);
}
Flow visitPublish(Publish* curr) { return Flow(NONCONSTANT_FLOW); }
Flow visitSIMDLoad(SIMDLoad* curr) { return Flow(NONCONSTANT_FLOW); }
Flow visitSIMDLoadSplat(SIMDLoad* curr) { return Flow(NONCONSTANT_FLOW); }
Flow visitSIMDLoadExtend(SIMDLoad* curr) { return Flow(NONCONSTANT_FLOW); }
Expand Down
1 change: 1 addition & 0 deletions src/wasm-ir-builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
Result<> makeStructWait(HeapType type, Index index);
Result<> makeWaitqueueNew();
Result<> makeWaitqueueNotify();
Result<> makePublish();
Result<> makeArrayNew(HeapType type);
Result<> makeArrayNewDefault(HeapType type);
Result<> makeArrayNewData(HeapType type, Name data);
Expand Down
11 changes: 11 additions & 0 deletions src/wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,7 @@ class Expression {
WideIntMulId,
WaitqueueNewId,
WaitqueueNotifyId,
PublishId,
NumExpressionIds
};
Id _id;
Expand Down Expand Up @@ -1842,6 +1843,16 @@ class WaitqueueNotify
void finalize();
};

class Publish : public SpecificExpression<Expression::PublishId> {
public:
Publish() = default;
Publish(MixedArena& allocator) : Publish() {}

Expression* ref;

void finalize();
};

class ArrayNew : public SpecificExpression<Expression::ArrayNewId> {
public:
ArrayNew() = default;
Expand Down
3 changes: 3 additions & 0 deletions src/wasm/wasm-binary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4182,6 +4182,9 @@ Result<> WasmBinaryReader::readInst() {
case BinaryConsts::WaitqueueNew: {
return builder.makeWaitqueueNew();
}
case BinaryConsts::Publish: {
return builder.makePublish();
}
}
return Err{"unknown atomic operation " + std::to_string(op)};
}
Expand Down
Loading
Loading