Asyncify-verbose: show multiple reasons why a function is processed #6457
Conversation
curiousdannii
changed the title
src/ir/module-utils.h
Outdated
| work.push(func.get()); | ||
| } | ||
| } | ||
| while (!work.empty()) { | ||
| auto* func = work.pop(); | ||
| for (auto* caller : map[func].calledBy) { | ||
| logVisit(map[caller], func); | ||
| // 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])) { |
There was a problem hiding this comment.
All of this code makes a lot of map lookups (map[caller]). Worth caching?
|
#6467 landed so merging in main here will add it. |
|
Great! I edited the test to test a function that calls two asyncified functions, so both callees should now be logged. |
|
It looks like a clang formatting error showed up on CI here. |
Head branch was pushed to by a user without write access
|
The asyncify_verbose.wast test is failing on the Windows builder on the main branch. Did this this commit perhaps introduce some non-determinism in the output? |
|
More likely that it exposed some non-determinism that already existed? I don't know how I'm pretty sure that this PR doesn't change how the tree is walked, only how it logs as it goes. But if it was being walked differently on different platforms, I'm a little surprised the non-determinism didn't present earlier, as that should've resulted in a different function being logged when only one function was logged... maybe before now there were no tests that had a diamond call tree? I did add that in this PR. |
|
@curiousdannii I think that's the explanation, yeah - the new test exposed existing nondeterminism. I opened #6479 |
#6457 added a test that exposed existing nondeterminism.
Partially solves emscripten-core/emscripten#17380
This doesn't actually make a tree, but it will at least show when a function calls more than one Asyncified function.