Skip to content

Commit 39cd787

Browse files
stevenfontanellastevenfontanella
committed
Acquire/release memory ordering for loads
1 parent 4236103 commit 39cd787

File tree

15 files changed

+323
-136
lines changed

15 files changed

+323
-136
lines changed

‎src/binaryen-c.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1344,14 +1344,16 @@ BinaryenExpressionRef BinaryenAtomicLoad(BinaryenModuleRef module,
13441344
uint32_t offset,
13451345
BinaryenType type,
13461346
BinaryenExpressionRef ptr,
1347-
const char* memoryName) {
1347+
const char* memoryName,
1348+
uint8_t order) {
13481349
return static_cast<Expression*>(
13491350
Builder(*(Module*)module)
13501351
.makeAtomicLoad(bytes,
13511352
offset,
13521353
(Expression*)ptr,
13531354
Type(type),
1354-
getMemoryName(module, memoryName)));
1355+
getMemoryName(module, memoryName),
1356+
static_cast<MemoryOrder>(order)));
13551357
}
13561358
BinaryenExpressionRef BinaryenAtomicStore(BinaryenModuleRef module,
13571359
uint32_t bytes,

‎src/binaryen-c.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -848,7 +848,8 @@ BINARYEN_API BinaryenExpressionRef BinaryenAtomicLoad(BinaryenModuleRef module,
848848
uint32_t offset,
849849
BinaryenType type,
850850
BinaryenExpressionRef ptr,
851-
const char* memoryName);
851+
const char* memoryName,
852+
uint8_t order);
852853
BINARYEN_API BinaryenExpressionRef
853854
BinaryenAtomicStore(BinaryenModuleRef module,
854855
uint32_t bytes,

‎src/parser/contexts.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,8 @@ struct NullInstrParserCtx {
555555
int,
556556
bool,
557557
MemoryIdxT*,
558-
MemargT) {
558+
MemargT,
559+
MemoryOrder) {
559560
return Ok{};
560561
}
561562
Result<> makeStore(Index,
@@ -2235,12 +2236,13 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx>, AnnotationParserCtx {
22352236
int bytes,
22362237
bool isAtomic,
22372238
Name* mem,
2238-
Memarg memarg) {
2239+
Memarg memarg,
2240+
MemoryOrder order) {
22392241
auto m = getMemory(pos, mem);
22402242
CHECK_ERR(m);
22412243
if (isAtomic) {
2242-
return withLoc(pos,
2243-
irBuilder.makeAtomicLoad(bytes, memarg.offset, type, *m));
2244+
return withLoc(
2245+
pos, irBuilder.makeAtomicLoad(bytes, memarg.offset, type, *m, order));
22442246
}
22452247
return withLoc(pos,
22462248
irBuilder.makeLoad(

‎src/parser/parsers.h‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ template<typename Ctx> Result<typename Ctx::MemTypeT> memtype(Ctx&);
4949
template<typename Ctx>
5050
Result<typename Ctx::MemTypeT> memtypeContinued(Ctx&, Type addressType);
5151
template<typename Ctx> Result<MemoryOrder> memorder(Ctx&);
52+
template<typename Ctx> MaybeResult<MemoryOrder> maybeMemOrder(Ctx&);
5253
template<typename Ctx> Result<typename Ctx::TableTypeT> tabletype(Ctx&);
5354
template<typename Ctx>
5455
Result<typename Ctx::TableTypeT> tabletypeContinued(Ctx&, Type addressType);
@@ -860,6 +861,18 @@ template<typename Ctx> Result<MemoryOrder> memorder(Ctx& ctx) {
860861
return MemoryOrder::SeqCst;
861862
}
862863

864+
// memorder ::= 'seqcst' | 'acqrel'
865+
template<typename Ctx> MaybeResult<MemoryOrder> maybeMemOrder(Ctx& ctx) {
866+
if (ctx.in.takeKeyword("seqcst"sv)) {
867+
return MemoryOrder::SeqCst;
868+
}
869+
if (ctx.in.takeKeyword("acqrel"sv)) {
870+
return MemoryOrder::AcqRel;
871+
}
872+
873+
return {};
874+
}
875+
863876
// tabletype ::= (limits32 | 'i32' limits32 | 'i64' limit64) reftype
864877
template<typename Ctx> Result<typename Ctx::TableTypeT> tabletype(Ctx& ctx) {
865878
Type addressType = Type::i32;
@@ -1737,12 +1750,30 @@ Result<> makeLoad(Ctx& ctx,
17371750
bool signed_,
17381751
int bytes,
17391752
bool isAtomic) {
1753+
1754+
// We could only parse this when `isAtomic`, but this way gives a clearer
1755+
// error since `memIdx` can never be mistaken for a `memOrder`.
1756+
auto maybeOrder = maybeMemOrder(ctx);
1757+
CHECK_ERR(maybeOrder);
1758+
1759+
if (maybeOrder && !isAtomic) {
1760+
return Err{"Memory ordering can only be provided for atomic loads."};
1761+
}
1762+
17401763
auto mem = maybeMemidx(ctx);
17411764
CHECK_ERR(mem);
17421765
auto arg = memarg(ctx, bytes);
17431766
CHECK_ERR(arg);
1744-
return ctx.makeLoad(
1745-
pos, annotations, type, signed_, bytes, isAtomic, mem.getPtr(), *arg);
1767+
// todo should we remove isAtomic?
1768+
return ctx.makeLoad(pos,
1769+
annotations,
1770+
type,
1771+
signed_,
1772+
bytes,
1773+
isAtomic,
1774+
mem.getPtr(),
1775+
*arg,
1776+
maybeOrder ? *maybeOrder : MemoryOrder::SeqCst);
17461777
}
17471778

17481779
template<typename Ctx>

‎src/passes/Print.cpp‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,7 @@ struct PrintExpressionContents
574574
}
575575
restoreNormalColor(o);
576576
printMemoryName(curr->memory, o, wasm);
577+
printMemoryOrderNoSpaces(curr->order, /*prependSpace=*/true);
577578
if (curr->offset) {
578579
o << " offset=" << curr->offset;
579580
}
@@ -2323,7 +2324,8 @@ struct PrintExpressionContents
23232324
o << index;
23242325
}
23252326
}
2326-
void printMemoryOrder(MemoryOrder order) {
2327+
2328+
void printMemoryOrderNoSpaces(MemoryOrder order, bool prependSpace) {
23272329
switch (order) {
23282330
// Unordered should have a different base instruction, so there is nothing
23292331
// to print. We could be explicit and print seqcst, but we choose not to
@@ -2332,10 +2334,16 @@ struct PrintExpressionContents
23322334
case MemoryOrder::SeqCst:
23332335
break;
23342336
case MemoryOrder::AcqRel:
2335-
o << "acqrel ";
2337+
o << (prependSpace ? " " : "") << "acqrel";
23362338
break;
23372339
}
23382340
}
2341+
2342+
void printMemoryOrder(MemoryOrder order) {
2343+
printMemoryOrderNoSpaces(order, false);
2344+
o << " ";
2345+
}
2346+
23392347
void visitStructGet(StructGet* curr) {
23402348
auto heapType = curr->ref->type.getHeapType();
23412349
const auto& field = heapType.getStruct().fields[curr->index];

‎src/tools/wasm-split/instrumenter.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,12 @@ void Instrumenter::addProfileExport(size_t numFuncs) {
287287
getAddr(),
288288
builder.makeBinary(
289289
MulInt32, getFuncIdx(), builder.makeConst(uint32_t(4)))),
290-
builder.makeAtomicLoad(
291-
1, 0, getFuncIdx(), Type::i32, loadMemoryName),
290+
builder.makeAtomicLoad(1,
291+
0,
292+
getFuncIdx(),
293+
Type::i32,
294+
loadMemoryName,
295+
MemoryOrder::SeqCst),
292296
Type::i32,
293297
wasm->memories[0]->name),
294298
builder.makeLocalSet(

‎src/wasm-binary.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,8 +1728,10 @@ class WasmBinaryReader {
17281728
size_t inlineHintsLen = 0;
17291729
void readInlineHints(size_t payloadLen);
17301730

1731-
Index readMemoryAccess(Address& alignment, Address& offset);
1732-
std::tuple<Name, Address, Address> getMemarg();
1731+
std::tuple<Address, Address, Index, MemoryOrder>
1732+
readMemoryAccess(bool isAtomic, bool isRMW);
1733+
std::tuple<Name, Address, Address, MemoryOrder> getMemarg(bool isAtomic,
1734+
bool isRMW);
17331735
MemoryOrder getMemoryOrder(bool isRMW = false);
17341736

17351737
[[noreturn]] void throwError(std::string text) {

‎src/wasm-builder.h‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,10 +384,16 @@ class Builder {
384384
ret->finalize();
385385
return ret;
386386
}
387-
Load* makeAtomicLoad(
388-
unsigned bytes, Address offset, Expression* ptr, Type type, Name memory) {
387+
Load* makeAtomicLoad(unsigned bytes,
388+
Address offset,
389+
Expression* ptr,
390+
Type type,
391+
Name memory,
392+
MemoryOrder order) {
389393
Load* load = makeLoad(bytes, false, offset, bytes, ptr, type, memory);
394+
// Probably should remove this?
390395
load->isAtomic = true;
396+
load->order = order;
391397
return load;
392398
}
393399
AtomicWait* makeAtomicWait(Expression* ptr,

‎src/wasm-ir-builder.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,8 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
156156
Name mem);
157157
Result<> makeStore(
158158
unsigned bytes, Address offset, unsigned align, Type type, Name mem);
159-
Result<> makeAtomicLoad(unsigned bytes, Address offset, Type type, Name mem);
159+
Result<> makeAtomicLoad(
160+
unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order);
160161
Result<> makeAtomicStore(unsigned bytes, Address offset, Type type, Name mem);
161162
Result<> makeAtomicRMW(
162163
AtomicRMWOp op, unsigned bytes, Address offset, Type type, Name mem);

‎src/wasm-stack.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,9 @@ class BinaryInstWriter : public OverriddenVisitor<BinaryInstWriter> {
129129
void emitMemoryAccess(size_t alignment,
130130
size_t bytes,
131131
uint64_t offset,
132-
Name memory);
132+
Name memory,
133+
MemoryOrder order,
134+
bool isRMW);
133135
int32_t getBreakIndex(Name name);
134136

135137
WasmBinaryWriter& parent;

0 commit comments

Comments
 (0)