Skip to content

Commit

Permalink
Refactor Host expression to MemorySize and MemoryGrow (#3137)
Browse files Browse the repository at this point in the history
Aligns the internal representations of `memory.size` and `memory.grow` with other more recent memory instructions by removing the legacy `Host` expression class and adding separate expression classes for `MemorySize` and `MemoryGrow`. Simplifies related APIs, but is also a breaking API change.
  • Loading branch information
dcodeIO authored Sep 17, 2020
1 parent e1d74ef commit 2841edd
Show file tree
Hide file tree
Showing 38 changed files with 402 additions and 482 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ Current Trunk
basic subtyping of `externref`, `funcref` and `exnref` (if enabled).
- Enabling the exception handling or anyref features without also enabling
reference types is a validation error now.
- The `Host` expression and its respective APIs have been refactored into
separate `MemorySize` and `MemoryGrow` expressions to align with other memory
instructions.

v96
---
Expand Down
4 changes: 2 additions & 2 deletions scripts/gen-s-parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@
("i64.store8", "makeStore(s, Type::i64, /*isAtomic=*/false)"),
("i64.store16", "makeStore(s, Type::i64, /*isAtomic=*/false)"),
("i64.store32", "makeStore(s, Type::i64, /*isAtomic=*/false)"),
("memory.size", "makeHost(s, HostOp::MemorySize)"),
("memory.grow", "makeHost(s, HostOp::MemoryGrow)"),
("memory.size", "makeMemorySize(s)"),
("memory.grow", "makeMemoryGrow(s)"),
("i32.const", "makeConst(s, Type::i32)"),
("i64.const", "makeConst(s, Type::i64)"),
("f32.const", "makeConst(s, Type::f32)"),
Expand Down
102 changes: 22 additions & 80 deletions src/binaryen-c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,12 @@ BinaryenExpressionId BinaryenBinaryId(void) { return Expression::Id::BinaryId; }
BinaryenExpressionId BinaryenSelectId(void) { return Expression::Id::SelectId; }
BinaryenExpressionId BinaryenDropId(void) { return Expression::Id::DropId; }
BinaryenExpressionId BinaryenReturnId(void) { return Expression::Id::ReturnId; }
BinaryenExpressionId BinaryenHostId(void) { return Expression::Id::HostId; }
BinaryenExpressionId BinaryenMemorySizeId(void) {
return Expression::Id::MemorySizeId;
}
BinaryenExpressionId BinaryenMemoryGrowId(void) {
return Expression::Id::MemoryGrowId;
}
BinaryenExpressionId BinaryenNopId(void) { return Expression::Id::NopId; }
BinaryenExpressionId BinaryenUnreachableId(void) {
return Expression::Id::UnreachableId;
Expand Down Expand Up @@ -509,8 +514,6 @@ BinaryenOp BinaryenLtFloat64(void) { return LtFloat64; }
BinaryenOp BinaryenLeFloat64(void) { return LeFloat64; }
BinaryenOp BinaryenGtFloat64(void) { return GtFloat64; }
BinaryenOp BinaryenGeFloat64(void) { return GeFloat64; }
BinaryenOp BinaryenMemorySize(void) { return MemorySize; }
BinaryenOp BinaryenMemoryGrow(void) { return MemoryGrow; }
BinaryenOp BinaryenAtomicRMWAdd(void) { return AtomicRMWOp::Add; }
BinaryenOp BinaryenAtomicRMWSub(void) { return AtomicRMWOp::Sub; }
BinaryenOp BinaryenAtomicRMWAnd(void) { return AtomicRMWOp::And; }
Expand Down Expand Up @@ -1056,20 +1059,13 @@ BinaryenExpressionRef BinaryenReturn(BinaryenModuleRef module,
auto* ret = Builder(*(Module*)module).makeReturn((Expression*)value);
return static_cast<Expression*>(ret);
}
BinaryenExpressionRef BinaryenHost(BinaryenModuleRef module,
BinaryenOp op,
const char* name,
BinaryenExpressionRef* operands,
BinaryenIndex numOperands) {
auto* ret = ((Module*)module)->allocator.alloc<Host>();
ret->op = HostOp(op);
if (name) {
ret->nameOperand = name;
}
for (BinaryenIndex i = 0; i < numOperands; i++) {
ret->operands.push_back((Expression*)operands[i]);
}
ret->finalize();
BinaryenExpressionRef BinaryenMemorySize(BinaryenModuleRef module) {
auto* ret = Builder(*(Module*)module).makeMemorySize();
return static_cast<Expression*>(ret);
}
BinaryenExpressionRef BinaryenMemoryGrow(BinaryenModuleRef module,
BinaryenExpressionRef delta) {
auto* ret = Builder(*(Module*)module).makeMemoryGrow((Expression*)delta);
return static_cast<Expression*>(ret);
}
BinaryenExpressionRef BinaryenNop(BinaryenModuleRef module) {
Expand Down Expand Up @@ -1826,72 +1822,18 @@ void BinaryenGlobalSetSetValue(BinaryenExpressionRef expr,
assert(valueExpr);
static_cast<GlobalSet*>(expression)->value = (Expression*)valueExpr;
}
// Host
BinaryenOp BinaryenHostGetOp(BinaryenExpressionRef expr) {
auto* expression = (Expression*)expr;
assert(expression->is<Host>());
return static_cast<Host*>(expression)->op;
}
void BinaryenHostSetOp(BinaryenExpressionRef expr, BinaryenOp op) {
auto* expression = (Expression*)expr;
assert(expression->is<Host>());
static_cast<Host*>(expression)->op = (HostOp)op;
}
const char* BinaryenHostGetNameOperand(BinaryenExpressionRef expr) {
auto* expression = (Expression*)expr;
assert(expression->is<Host>());
return static_cast<Host*>(expression)->nameOperand.c_str();
}
void BinaryenHostSetNameOperand(BinaryenExpressionRef expr, const char* name) {
auto* expression = (Expression*)expr;
assert(expression->is<Host>());
static_cast<Host*>(expression)->nameOperand = name ? name : "";
}
BinaryenIndex BinaryenHostGetNumOperands(BinaryenExpressionRef expr) {
auto* expression = (Expression*)expr;
assert(expression->is<Host>());
return static_cast<Host*>(expression)->operands.size();
}
BinaryenExpressionRef BinaryenHostGetOperandAt(BinaryenExpressionRef expr,
BinaryenIndex index) {
// MemoryGrow
BinaryenExpressionRef BinaryenMemoryGrowGetDelta(BinaryenExpressionRef expr) {
auto* expression = (Expression*)expr;
assert(expression->is<Host>());
assert(index < static_cast<Host*>(expression)->operands.size());
return static_cast<Host*>(expression)->operands[index];
assert(expression->is<MemoryGrow>());
return static_cast<MemoryGrow*>(expression)->delta;
}
void BinaryenHostSetOperandAt(BinaryenExpressionRef expr,
BinaryenIndex index,
BinaryenExpressionRef operandExpr) {
auto* expression = (Expression*)expr;
assert(expression->is<Host>());
assert(index < static_cast<Host*>(expression)->operands.size());
assert(operandExpr);
static_cast<Host*>(expression)->operands[index] = (Expression*)operandExpr;
}
BinaryenIndex BinaryenHostAppendOperand(BinaryenExpressionRef expr,
BinaryenExpressionRef operandExpr) {
auto* expression = (Expression*)expr;
assert(expression->is<Host>());
assert(operandExpr);
auto& list = static_cast<Host*>(expression)->operands;
auto index = list.size();
list.push_back((Expression*)operandExpr);
return index;
}
void BinaryenHostInsertOperandAt(BinaryenExpressionRef expr,
BinaryenIndex index,
BinaryenExpressionRef operandExpr) {
auto* expression = (Expression*)expr;
assert(expression->is<Host>());
assert(operandExpr);
static_cast<Host*>(expression)
->operands.insertAt(index, (Expression*)operandExpr);
}
BinaryenExpressionRef BinaryenHostRemoveOperandAt(BinaryenExpressionRef expr,
BinaryenIndex index) {
void BinaryenMemoryGrowSetDelta(BinaryenExpressionRef expr,
BinaryenExpressionRef deltaExpr) {
auto* expression = (Expression*)expr;
assert(expression->is<Host>());
return static_cast<Host*>(expression)->operands.removeAt(index);
assert(expression->is<MemoryGrow>());
assert(deltaExpr);
static_cast<MemoryGrow*>(expression)->delta = (Expression*)deltaExpr;
}
// Load
int BinaryenLoadIsAtomic(BinaryenExpressionRef expr) {
Expand Down
57 changes: 12 additions & 45 deletions src/binaryen-c.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ BINARYEN_API BinaryenExpressionId BinaryenBinaryId(void);
BINARYEN_API BinaryenExpressionId BinaryenSelectId(void);
BINARYEN_API BinaryenExpressionId BinaryenDropId(void);
BINARYEN_API BinaryenExpressionId BinaryenReturnId(void);
BINARYEN_API BinaryenExpressionId BinaryenHostId(void);
BINARYEN_API BinaryenExpressionId BinaryenMemorySizeId(void);
BINARYEN_API BinaryenExpressionId BinaryenMemoryGrowId(void);
BINARYEN_API BinaryenExpressionId BinaryenNopId(void);
BINARYEN_API BinaryenExpressionId BinaryenUnreachableId(void);
BINARYEN_API BinaryenExpressionId BinaryenAtomicCmpxchgId(void);
Expand Down Expand Up @@ -383,8 +384,6 @@ BINARYEN_API BinaryenOp BinaryenLtFloat64(void);
BINARYEN_API BinaryenOp BinaryenLeFloat64(void);
BINARYEN_API BinaryenOp BinaryenGtFloat64(void);
BINARYEN_API BinaryenOp BinaryenGeFloat64(void);
BINARYEN_API BinaryenOp BinaryenMemorySize(void);
BINARYEN_API BinaryenOp BinaryenMemoryGrow(void);
BINARYEN_API BinaryenOp BinaryenAtomicRMWAdd(void);
BINARYEN_API BinaryenOp BinaryenAtomicRMWSub(void);
BINARYEN_API BinaryenOp BinaryenAtomicRMWAnd(void);
Expand Down Expand Up @@ -729,12 +728,9 @@ BINARYEN_API BinaryenExpressionRef BinaryenDrop(BinaryenModuleRef module,
// Return: value can be NULL
BINARYEN_API BinaryenExpressionRef BinaryenReturn(BinaryenModuleRef module,
BinaryenExpressionRef value);
// Host: name may be NULL
BINARYEN_API BinaryenExpressionRef BinaryenHost(BinaryenModuleRef module,
BinaryenOp op,
const char* name,
BinaryenExpressionRef* operands,
BinaryenIndex numOperands);
BINARYEN_API BinaryenExpressionRef BinaryenMemorySize(BinaryenModuleRef module);
BINARYEN_API BinaryenExpressionRef
BinaryenMemoryGrow(BinaryenModuleRef module, BinaryenExpressionRef delta);
BINARYEN_API BinaryenExpressionRef BinaryenNop(BinaryenModuleRef module);
BINARYEN_API BinaryenExpressionRef
BinaryenUnreachable(BinaryenModuleRef module);
Expand Down Expand Up @@ -1153,43 +1149,14 @@ BinaryenGlobalSetGetValue(BinaryenExpressionRef expr);
BINARYEN_API void BinaryenGlobalSetSetValue(BinaryenExpressionRef expr,
BinaryenExpressionRef valueExpr);

// Host

// Gets the operation being performed by a host expression.
BINARYEN_API BinaryenOp BinaryenHostGetOp(BinaryenExpressionRef expr);
// Sets the operation being performed by a host expression.
BINARYEN_API void BinaryenHostSetOp(BinaryenExpressionRef expr, BinaryenOp op);
// Gets the name operand, if any, of a host expression.
BINARYEN_API const char* BinaryenHostGetNameOperand(BinaryenExpressionRef expr);
// Sets the name operand, if any, of a host expression.
BINARYEN_API void BinaryenHostSetNameOperand(BinaryenExpressionRef expr,
const char* nameOperand);
// Gets the number of operands of a host expression.
BINARYEN_API BinaryenIndex
BinaryenHostGetNumOperands(BinaryenExpressionRef expr);
// Gets the operand at the specified index of a host expression.
BINARYEN_API BinaryenExpressionRef
BinaryenHostGetOperandAt(BinaryenExpressionRef expr, BinaryenIndex index);
// Sets the operand at the specified index of a host expression.
BINARYEN_API void BinaryenHostSetOperandAt(BinaryenExpressionRef expr,
BinaryenIndex index,
BinaryenExpressionRef operandExpr);
// Appends an operand expression to a host expression, returning its insertion
// index.
BINARYEN_API BinaryenIndex BinaryenHostAppendOperand(
BinaryenExpressionRef expr, BinaryenExpressionRef operandExpr);
// Inserts an operand expression at the specified index of a host expression,
// moving existing operands including the one previously at that index one index
// up.
BINARYEN_API void
BinaryenHostInsertOperandAt(BinaryenExpressionRef expr,
BinaryenIndex index,
BinaryenExpressionRef operandExpr);
// Removes the operand expression at the specified index of a host expression,
// moving all subsequent operands one index down. Returns the operand
// expression.
// MemoryGrow

// Gets the delta of a `memory.grow` expression.
BINARYEN_API BinaryenExpressionRef
BinaryenHostRemoveOperandAt(BinaryenExpressionRef expr, BinaryenIndex index);
BinaryenMemoryGrowGetDelta(BinaryenExpressionRef expr);
// Sets the delta of a `memory.grow` expression.
BINARYEN_API void BinaryenMemoryGrowSetDelta(BinaryenExpressionRef expr,
BinaryenExpressionRef delta);

// Load

Expand Down
4 changes: 2 additions & 2 deletions src/gen-s-parser.inc
Original file line number Diff line number Diff line change
Expand Up @@ -2503,13 +2503,13 @@ switch (op[0]) {
if (strcmp(op, "memory.fill") == 0) { return makeMemoryFill(s); }
goto parse_error;
case 'g':
if (strcmp(op, "memory.grow") == 0) { return makeHost(s, HostOp::MemoryGrow); }
if (strcmp(op, "memory.grow") == 0) { return makeMemoryGrow(s); }
goto parse_error;
case 'i':
if (strcmp(op, "memory.init") == 0) { return makeMemoryInit(s); }
goto parse_error;
case 's':
if (strcmp(op, "memory.size") == 0) { return makeHost(s, HostOp::MemorySize); }
if (strcmp(op, "memory.size") == 0) { return makeMemorySize(s); }
goto parse_error;
default: goto parse_error;
}
Expand Down
6 changes: 2 additions & 4 deletions src/ir/ExpressionAnalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,8 @@ template<typename T> void visitImmediates(Expression* curr, T& visitor) {
void visitSelect(Select* curr) {}
void visitDrop(Drop* curr) {}
void visitReturn(Return* curr) {}
void visitHost(Host* curr) {
visitor.visitInt(curr->op);
visitor.visitNonScopeName(curr->nameOperand);
}
void visitMemorySize(MemorySize* curr) {}
void visitMemoryGrow(MemoryGrow* curr) {}
void visitRefNull(RefNull* curr) { visitor.visitType(curr->type); }
void visitRefIsNull(RefIsNull* curr) {}
void visitRefFunc(RefFunc* curr) { visitor.visitNonScopeName(curr->func); }
Expand Down
13 changes: 5 additions & 8 deletions src/ir/ExpressionManipulator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,11 @@ flexibleCopy(Expression* original, Module& wasm, CustomCopier custom) {
Expression* visitReturn(Return* curr) {
return builder.makeReturn(copy(curr->value));
}
Expression* visitHost(Host* curr) {
std::vector<Expression*> operands;
for (Index i = 0; i < curr->operands.size(); i++) {
operands.push_back(copy(curr->operands[i]));
}
auto* ret =
builder.makeHost(curr->op, curr->nameOperand, std::move(operands));
return ret;
Expression* visitMemorySize(MemorySize* curr) {
return builder.makeMemorySize();
}
Expression* visitMemoryGrow(MemoryGrow* curr) {
return builder.makeMemoryGrow(copy(curr->delta));
}
Expression* visitRefNull(RefNull* curr) {
return builder.makeRefNull(curr->type);
Expand Down
3 changes: 2 additions & 1 deletion src/ir/ReFinalize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ void ReFinalize::visitBinary(Binary* curr) { curr->finalize(); }
void ReFinalize::visitSelect(Select* curr) { curr->finalize(); }
void ReFinalize::visitDrop(Drop* curr) { curr->finalize(); }
void ReFinalize::visitReturn(Return* curr) { curr->finalize(); }
void ReFinalize::visitHost(Host* curr) { curr->finalize(); }
void ReFinalize::visitMemorySize(MemorySize* curr) { curr->finalize(); }
void ReFinalize::visitMemoryGrow(MemoryGrow* curr) { curr->finalize(); }
void ReFinalize::visitRefNull(RefNull* curr) { curr->finalize(); }
void ReFinalize::visitRefIsNull(RefIsNull* curr) { curr->finalize(); }
void ReFinalize::visitRefFunc(RefFunc* curr) { curr->finalize(); }
Expand Down
3 changes: 2 additions & 1 deletion src/ir/cost.h
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,8 @@ struct CostAnalyzer : public Visitor<CostAnalyzer, Index> {
}
Index visitDrop(Drop* curr) { return visit(curr->value); }
Index visitReturn(Return* curr) { return maybeVisit(curr->value); }
Index visitHost(Host* curr) { return 100; }
Index visitMemorySize(MemorySize* curr) { return 1; }
Index visitMemoryGrow(MemoryGrow* curr) { return 100; }
Index visitRefNull(RefNull* curr) { return 1; }
Index visitRefIsNull(RefIsNull* curr) { return 1; }
Index visitRefFunc(RefFunc* curr) { return 1; }
Expand Down
21 changes: 16 additions & 5 deletions src/ir/effects.h
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,8 @@ struct EffectAnalyzer
implicitTrap = true;
break;
}
default: {}
default: {
}
}
}
}
Expand All @@ -446,17 +447,27 @@ struct EffectAnalyzer
implicitTrap = true;
break;
}
default: {}
default: {
}
}
}
}
void visitSelect(Select* curr) {}
void visitDrop(Drop* curr) {}
void visitReturn(Return* curr) { branchesOut = true; }
void visitHost(Host* curr) {
void visitMemorySize(MemorySize* curr) {
// memory.size accesses the size of the memory, and thus can be modeled as
// reading memory
readsMemory = true;
// Atomics are sequentially consistent with memory.size.
isAtomic = true;
}
void visitMemoryGrow(MemoryGrow* curr) {
calls = true;
// memory.grow modifies the set of valid addresses, and thus can be modeled
// as modifying memory
// memory.grow technically does a read-modify-write operation on the memory
// size in the successful case, modifying the set of valid addresses, and
// just a read operation in the failure case
readsMemory = true;
writesMemory = true;
// Atomics are also sequentially consistent with memory.grow.
isAtomic = true;
Expand Down
6 changes: 4 additions & 2 deletions src/ir/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ struct ReFinalize
void visitSelect(Select* curr);
void visitDrop(Drop* curr);
void visitReturn(Return* curr);
void visitHost(Host* curr);
void visitMemorySize(MemorySize* curr);
void visitMemoryGrow(MemoryGrow* curr);
void visitRefNull(RefNull* curr);
void visitRefIsNull(RefIsNull* curr);
void visitRefFunc(RefFunc* curr);
Expand Down Expand Up @@ -213,7 +214,8 @@ struct ReFinalizeNode : public OverriddenVisitor<ReFinalizeNode> {
void visitSelect(Select* curr) { curr->finalize(); }
void visitDrop(Drop* curr) { curr->finalize(); }
void visitReturn(Return* curr) { curr->finalize(); }
void visitHost(Host* curr) { curr->finalize(); }
void visitMemorySize(MemorySize* curr) { curr->finalize(); }
void visitMemoryGrow(MemoryGrow* curr) { curr->finalize(); }
void visitRefNull(RefNull* curr) { curr->finalize(); }
void visitRefIsNull(RefIsNull* curr) { curr->finalize(); }
void visitRefFunc(RefFunc* curr) { curr->finalize(); }
Expand Down
Loading

0 comments on commit 2841edd

Please sign in to comment.