Update LogExecution pass to emit log ordinals in the wasm function ordinals order#5718
Update LogExecution pass to emit log ordinals in the wasm function ordinals order#5718juj wants to merge 9 commits intoWebAssembly:mainfrom
Conversation
…dinals order, so that they can be correlated with functions afterwards.
| if (func->imported()) | ||
| continue; | ||
|
|
||
| Index currentFunctionIndex = (Index)stringToIndex(func->name.toString().c_str()); |
There was a problem hiding this comment.
| Index currentFunctionIndex = (Index)stringToIndex(func->name.toString().c_str()); | |
| Index currentFunctionIndex = stringToIndex(func->name.toString().c_str()); |
Also, why try to convert function names to indices at all? It would be more robust to avoid trying to interpret the names so that all function names can be treated uniformly.
There was a problem hiding this comment.
Hey @tlively, it's been a while, though I am now coming back to this functionality, and look to get this landed.
Updated the PR here: the idea here is to ensure that Wasm functions that do not have a debug symbol name string, would be guaranteed to get the log ID that matches its function ordinal. It gives consistency, and allows users to map the log IDs then to e.g. with external symbol maps.
Revised the PR to make sure this is the case always, even if some producer might e.g. give debug symbol names to only some of the functions, and not all.
# Conflicts: # src/passes/LogExecution.cpp
|
Updated this PR to latest, looking for a review. Thanks! |
|
@kripken Maybe you might be available to review? |
| Name LOGGER("log_execution"); | ||
|
|
||
| struct LogExecution : public WalkerPass<PostWalker<LogExecution>> { | ||
| std::unordered_map<Function*, Index> functionOrdinals; |
There was a problem hiding this comment.
What is a "function ordinal"? I don't see "ordinal" anywhere existing in src/.
If it is a new concept please add a comment explaining it.
If this is a function index, please rename it to that. Skimming the code below that might be the case?
There was a problem hiding this comment.
They are the function indices. Good idea, I'll rename.
I'll post this in a bit, I find there is some kind of bug in my use, and not sure if it could be related to this code path; so I'll investigate that and then update this PR.
3 participants
Update LogExecution pass to emit log ordinals in the wasm function ordinals order, so that they can be correlated with functions afterwards.
This is something I found I needed to do in order to get a PGO based analysis to line up with wasm function indices.