Skip to content

Commit

Permalink
Asyncify-verbose: Show all reasons why a function is instrumented (#6457
Browse files Browse the repository at this point in the history
)

Helps emscripten-core/emscripten#17380 by logging all the reasons why we
instrument a function, and not just the first as we did before.
  • Loading branch information
curiousdannii authored Apr 8, 2024
1 parent 98f08ef commit f0dd994
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 16 deletions.
24 changes: 16 additions & 8 deletions src/ir/module-utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -390,29 +390,37 @@ template<typename T> struct CallGraphPropertyAnalysis {
//
// 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.
// addProperty() - Adds the property.
// logVisit() - Log each visit of the propagation. This is called before
// we check if the function already has the property.
void propagateBack(std::function<bool(const T&)> hasProperty,
std::function<bool(const T&)> canHaveProperty,
std::function<void(T&, Function*)> addProperty,
std::function<void(T&)> addProperty,
std::function<void(const T&, Function*)> logVisit,
NonDirectCalls nonDirectCalls) {
// 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()]) ||
(nonDirectCalls == NonDirectCallsHaveProperty &&
map[func.get()].hasNonDirectCall)) {
addProperty(map[func.get()], func.get());
addProperty(map[func.get()]);
work.push(func.get());
}
}
while (!work.empty()) {
auto* func = work.pop();
for (auto* caller : map[func].calledBy) {
// 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], func);
// Skip functions forbidden from getting this property.
if (!canHaveProperty(map[caller])) {
continue;
}
// Log now, even if the function already has the property.
logVisit(map[caller], func);
// If we don't already have the property, then add it now, and propagate
// further.
if (!hasProperty(map[caller])) {
addProperty(map[caller]);
work.push(caller);
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/passes/Asyncify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -716,13 +716,13 @@ class ModuleAnalyzer {
return !info.isBottomMostRuntime &&
!info.inRemoveList;
},
[verbose](Info& info, Function* reason) {
if (verbose && !info.canChangeState) {
[](Info& info) { info.canChangeState = true; },
[verbose](const Info& info, Function* reason) {
if (verbose) {
std::cout << "[asyncify] " << info.name
<< " can change the state due to "
<< reason->name << "\n";
}
info.canChangeState = true;
},
scanner.IgnoreNonDirectCalls);

Expand Down
10 changes: 5 additions & 5 deletions src/passes/PostEmscripten.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -287,11 +287,11 @@ struct PostEmscripten : public Pass {
});

// Assume a non-direct call might throw.
analyzer.propagateBack(
[](const Info& info) { return info.canThrow; },
[](const Info& info) { return true; },
[](Info& info, Function* reason) { info.canThrow = true; },
analyzer.NonDirectCallsHaveProperty);
analyzer.propagateBack([](const Info& info) { return info.canThrow; },
[](const Info& info) { return true; },
[](Info& info) { info.canThrow = true; },
[](const Info& info, Function* reason) {},
analyzer.NonDirectCallsHaveProperty);

// Apply the information.
struct OptimizeInvokes : public WalkerPass<PostWalker<OptimizeInvokes>> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
;; Likewise, further up the call chain as well.
;;
;; CHECK: [asyncify] calls-calls-import can change the state due to calls-import
;; CHECK: [asyncify] calls-calls-import-b can change the state due to calls-import
;; CHECK: [asyncify] calls-calls-calls-import can change the state due to calls-calls-import
;; CHECK: [asyncify] calls-calls-calls-import can change the state due to calls-calls-import-b

(module
(import "env" "import" (func $import))
Expand All @@ -26,8 +28,13 @@
(call $calls-import)
)

(func $calls-calls-import-b
(call $calls-import)
)

(func $calls-calls-calls-import
(call $calls-calls-import)
(call $calls-calls-import-b)
)

(func $nothing
Expand Down

0 comments on commit f0dd994

Please # to comment.