Skip to content

Handle drops of unknown values in ExpressionRunner #2787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed
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
19 changes: 14 additions & 5 deletions src/ir/effects.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,22 @@ namespace wasm {

struct EffectAnalyzer
: public PostWalker<EffectAnalyzer, OverriddenVisitor<EffectAnalyzer>> {
EffectAnalyzer(const PassOptions& passOptions,
EffectAnalyzer(bool ignoreImplicitTraps,
bool debugInfo,
FeatureSet features,
Expression* ast = nullptr)
: ignoreImplicitTraps(passOptions.ignoreImplicitTraps),
debugInfo(passOptions.debugInfo), features(features) {
: ignoreImplicitTraps(ignoreImplicitTraps), debugInfo(debugInfo),
features(features) {
if (ast) {
analyze(ast);
}
}
EffectAnalyzer(const PassOptions& passOptions,
FeatureSet features,
Expression* ast = nullptr)
: EffectAnalyzer(
passOptions.ignoreImplicitTraps, passOptions.debugInfo, features, ast) {
}

bool ignoreImplicitTraps;
bool debugInfo;
Expand Down Expand Up @@ -400,7 +407,8 @@ struct EffectAnalyzer
implicitTrap = true;
break;
}
default: {}
default: {
}
}
}
}
Expand All @@ -418,7 +426,8 @@ struct EffectAnalyzer
implicitTrap = true;
break;
}
default: {}
default: {
}
}
}
}
Expand Down
27 changes: 27 additions & 0 deletions src/wasm-interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -1305,6 +1305,10 @@ class ConstantExpressionRunner : public ExpressionRunner<SubType> {
// Map remembering concrete global values set in the context of this flow.
std::unordered_map<Name, Literals> globalValues;

// Whether an unknown value of a local or global has been encountered in a
// sub-expression.
bool seenUnknownValue = false;

public:
struct NonconstantException {
}; // TODO: use a flow with a special name, as this is likely very slow
Expand Down Expand Up @@ -1339,6 +1343,7 @@ class ConstantExpressionRunner : public ExpressionRunner<SubType> {
if (iter != localValues.end()) {
return Flow(iter->second);
}
seenUnknownValue = true;
return Flow(NONCONSTANT_FLOW);
}
Flow visitLocalSet(LocalSet* curr) {
Expand Down Expand Up @@ -1375,6 +1380,7 @@ class ConstantExpressionRunner : public ExpressionRunner<SubType> {
if (iter != globalValues.end()) {
return Flow(iter->second);
}
seenUnknownValue = true;
return Flow(NONCONSTANT_FLOW);
}
Flow visitGlobalSet(GlobalSet* curr) {
Expand All @@ -1393,6 +1399,27 @@ class ConstantExpressionRunner : public ExpressionRunner<SubType> {
}
return Flow(NONCONSTANT_FLOW);
}
Flow visitDrop(Drop* curr) {
NOTE_ENTER("Drop");
seenUnknownValue = false;
Flow value = ExpressionRunner<SubType>::visit(curr->value);
if (value.breaking()) {
// Handle the case where all we don't know to perform the drop is a local
// or global value, which we do not need to know if there are no other
// side-effects.
if (seenUnknownValue && value.breakTo == NONCONSTANT_FLOW &&
module != nullptr) {
EffectAnalyzer effects(false, false, module->features, curr->value);
effects.localsRead.clear();
effects.globalsRead.clear();
if (!effects.hasAnything()) {
return Flow();
}
}
return value;
}
return Flow();
}
Flow visitCall(Call* curr) {
NOTE_ENTER("Call");
NOTE_NAME(curr->target);
Expand Down
22 changes: 22 additions & 0 deletions test/binaryen.js/expressionrunner.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,5 +204,27 @@ expr = runner.runAndDispose(
);
assert(expr === 0);

// Should not bail on dropped expressions if all we don't know is a value
runner = new binaryen.ExpressionRunner(module);
expr = runner.runAndDispose(
module.block(null, [
module.drop(
module.i32.add(
module.local.get(0, binaryen.i32), // here
module.global.get("aGlobal", binaryen.i32), //
)
),
module.i32.const(9)
], binaryen.i32)
);
assertDeepEqual(
binaryen.getExpressionInfo(expr),
{
id: binaryen.ExpressionIds.Const,
type: binaryen.i32,
value: 9
}
);

module.dispose();
binaryen.setAPITracing(false);
14 changes: 14 additions & 0 deletions test/binaryen.js/expressionrunner.js.txt
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,20 @@ int main() {
expressions[62] = BinaryenBreak(the_module, "theLoop", expressions[0], expressions[0]);
expressions[63] = BinaryenLoop(the_module, "theLoop", expressions[62]);
ExpressionRunnerRunAndDispose(expressionRunners[11], expressions[63]);
expressionRunners[12] = ExpressionRunnerCreate(the_module, 0, 0, 0);
expressions[64] = BinaryenLocalGet(the_module, 0, BinaryenTypeInt32());
expressions[65] = BinaryenGlobalGet(the_module, "aGlobal", BinaryenTypeInt32());
expressions[66] = BinaryenBinary(the_module, 0, expressions[64], expressions[65]);
expressions[67] = BinaryenDrop(the_module, expressions[66]);
expressions[68] = BinaryenConst(the_module, BinaryenLiteralInt32(9));
{
BinaryenExpressionRef children[] = { expressions[67], expressions[68] };
expressions[69] = BinaryenBlock(the_module, NULL, children, 2, BinaryenTypeInt32());
}
expressions[70] = ExpressionRunnerRunAndDispose(expressionRunners[12], expressions[69]);
BinaryenExpressionGetId(expressions[70]);
BinaryenExpressionGetType(expressions[70]);
BinaryenConstGetValueI32(expressions[70]);
BinaryenModuleDispose(the_module);
types.clear();
expressions.clear();
Expand Down
7 changes: 1 addition & 6 deletions test/passes/precompute_all-features.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,7 @@
(i32.const 2300)
)
(nop)
(drop
(i32.add
(i32.const 1)
(local.get $x)
)
)
(nop)
(nop)
(nop)
(nop)
Expand Down