Skip to content
Merged
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
11 changes: 8 additions & 3 deletions src/ir/module-utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -349,17 +349,22 @@ template<typename T> struct CallGraphPropertyAnalysis {
enum IndirectCalls { IgnoreIndirectCalls, IndirectCallsHaveProperty };

// Propagate a property from a function to those that call it.
//
// hasProperty() - Check if the property is present.
// canHaveProperty() - Check if the property could be present.
// addProperty() - Adds the property. This receives a second parameter which
// is the function due to which we are adding the property.
void propagateBack(std::function<bool(const T&)> hasProperty,
std::function<bool(const T&)> canHaveProperty,
std::function<void(T&)> addProperty,
std::function<void(T&, Function*)> addProperty,
IndirectCalls indirectCalls) {
// The work queue contains items we just learned can change the state.
UniqueDeferredQueue<Function*> work;
for (auto& func : wasm.functions) {
if (hasProperty(map[func.get()]) ||
(indirectCalls == IndirectCallsHaveProperty &&
map[func.get()].hasIndirectCall)) {
addProperty(map[func.get()]);
addProperty(map[func.get()], func.get());
work.push(func.get());
}
}
Expand All @@ -369,7 +374,7 @@ template<typename T> struct CallGraphPropertyAnalysis {
// If we don't already have the property, and we are not forbidden
// from getting it, then it propagates back to us now.
if (!hasProperty(map[caller]) && canHaveProperty(map[caller])) {
addProperty(map[caller]);
addProperty(map[caller], func);
work.push(caller);
}
}
Expand Down
49 changes: 45 additions & 4 deletions src/passes/Asyncify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,11 @@
// an unwind/rewind in an invalid place (this can be helpful for manual
// tweaking of the only-list / remove-list, see later).
//
// --pass-arg=asyncify-verbose
//
// Logs out instrumentation decisions to the console. This can help figure
// out why a certain function was instrumented.
//
// For manual fine-tuning of the list of instrumented functions, there are lists
// that you can set. These must be used carefully, as misuse can break your
// application - for example, if a function is called that should be
Expand Down Expand Up @@ -486,6 +491,8 @@ class ModuleAnalyzer {

struct Info
: public ModuleUtils::CallGraphPropertyAnalysis<Info>::FunctionInfo {
// The function name.
Name name;
// If this function can start an unwind/rewind.
bool canChangeState = false;
// If this function is part of the runtime that receives an unwinding
Expand Down Expand Up @@ -513,9 +520,10 @@ class ModuleAnalyzer {
const String::Split& removeListInput,
const String::Split& addListInput,
const String::Split& onlyListInput,
bool asserts)
bool asserts,
bool verbose)
: module(module), canIndirectChangeState(canIndirectChangeState),
fakeGlobals(module), asserts(asserts) {
fakeGlobals(module), asserts(asserts), verbose(verbose) {

PatternMatcher removeList("remove", module, removeListInput);
PatternMatcher addList("add", module, addListInput);
Expand Down Expand Up @@ -548,6 +556,7 @@ class ModuleAnalyzer {
// name.
ModuleUtils::CallGraphPropertyAnalysis<Info> scanner(
module, [&](Function* func, Info& info) {
info.name = func->name;
if (func->imported()) {
// The relevant asyncify imports can definitely change the state.
if (func->module == ASYNCIFY &&
Expand All @@ -556,6 +565,10 @@ class ModuleAnalyzer {
} else {
info.canChangeState =
canImportChangeState(func->module, func->base);
if (verbose && info.canChangeState) {
std::cout << "[asyncify] " << func->name
<< " is an import that can change the state\n";
}
}
return;
}
Expand Down Expand Up @@ -609,6 +622,10 @@ class ModuleAnalyzer {
// the bottom-most runtime also doing top-most runtime stuff
// like starting and unwinding.
}
if (verbose && info.canChangeState) {
std::cout << "[asyncify] " << func->name
<< " can change the state due to initial scan\n";
}
});

// Functions in the remove-list are assumed to not change the state.
Expand All @@ -617,6 +634,10 @@ class ModuleAnalyzer {
auto& info = pair.second;
if (removeList.match(func->name)) {
info.inRemoveList = true;
if (verbose && info.canChangeState) {
std::cout << "[asyncify] " << func->name
<< " is in the remove-list, ignore\n";
}
info.canChangeState = false;
}
}
Expand Down Expand Up @@ -648,7 +669,14 @@ class ModuleAnalyzer {
return !info.isBottomMostRuntime &&
!info.inRemoveList;
},
[](Info& info) { info.canChangeState = true; },
[verbose](Info& info, Function* reason) {
if (verbose && !info.canChangeState) {
std::cout << "[asyncify] " << info.name
<< " can change the state due to "
<< reason->name << "\n";
}
info.canChangeState = true;
},
scanner.IgnoreIndirectCalls);

map.swap(scanner.map);
Expand All @@ -663,6 +691,11 @@ class ModuleAnalyzer {
if (matched) {
info.addedFromList = true;
}
if (verbose) {
std::cout << "[asyncify] " << func->name
<< "'s state is set based on the only-list to " << matched
<< '\n';
}
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.

Is looks possible for some functions to be logged as added as can-change-state functions but if the only-list is present, they are eventually ignored silently, i.e., without any other log messages that they are ignored, right? Would it be better to check the only list when logging for other causes..?

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.

I think this log message here would be shown, though, so it's not completely silent? This message would say they are set to false, which overrides the previous messages.

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.

Ah you're right. Sorry I thought this is only printed when the value is true.

}
}
}
Expand All @@ -671,6 +704,10 @@ class ModuleAnalyzer {
for (auto& func : module.functions) {
if (!func->imported() && addList.match(func->name)) {
auto& info = map[func.get()];
if (verbose && !info.canChangeState) {
std::cout << "[asyncify] " << func->name
<< " is in the add-list, add\n";
}
info.canChangeState = true;
info.addedFromList = true;
}
Expand Down Expand Up @@ -741,6 +778,7 @@ class ModuleAnalyzer {

FakeGlobalHelper fakeGlobals;
bool asserts;
bool verbose;
};

// Checks if something performs a call: either a direct or indirect call,
Expand Down Expand Up @@ -1397,6 +1435,8 @@ struct Asyncify : public Pass {
String::trim(read_possible_response_file(onlyListInput)), ",");
auto asserts =
runner->options.getArgumentOrDefault("asyncify-asserts", "") != "";
auto verbose =
runner->options.getArgumentOrDefault("asyncify-verbose", "") != "";

removeList = handleBracketingOperators(removeList);
addList = handleBracketingOperators(addList);
Expand Down Expand Up @@ -1427,7 +1467,8 @@ struct Asyncify : public Pass {
removeList,
addList,
onlyList,
asserts);
asserts,
verbose);

// Add necessary globals before we emit code to use them.
addGlobals(module);
Expand Down
9 changes: 5 additions & 4 deletions src/passes/PostEmscripten.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,11 @@ struct PostEmscripten : public Pass {
});

// Assume an indirect call might throw.
analyzer.propagateBack([](const Info& info) { return info.canThrow; },
[](const Info& info) { return true; },
[](Info& info) { info.canThrow = true; },
analyzer.IndirectCallsHaveProperty);
analyzer.propagateBack(
[](const Info& info) { return info.canThrow; },
[](const Info& info) { return true; },
[](Info& info, Function* reason) { info.canThrow = true; },
analyzer.IndirectCallsHaveProperty);

// Apply the information.
struct OptimizeInvokes : public WalkerPass<PostWalker<OptimizeInvokes>> {
Expand Down
Loading