Heap2Local: reset data structures after optimizing#8857
Conversation
This entirely eliminates the class of bugs due to stale data in the `parents` and `localGraph` after a previous allocation has been optimized.
kripken
left a comment
There was a problem hiding this comment.
I think this PR is the way to go. It is just too risky to keep such data structures up to date. As optimizing is not super-common here, I think the overhead is probably acceptable.
| localGraph = std::make_unique<LazyLocalGraph>(func, &wasm); | ||
| parents = std::make_unique<Parents>(func->body); | ||
| branchTargets = | ||
| std::make_unique<BranchUtils::BranchTargets>(func->body); |
There was a problem hiding this comment.
Can have a helper for this repeating logic.
There was a problem hiding this comment.
Now we can optimize more, as we notice that we compare to a descriptor created in this function.
| (func $cmpxchg-expected-first | ||
| (drop | ||
| (struct.atomic.rmw.cmpxchg $struct 0 | ||
| (struct.new_default $struct) |
There was a problem hiding this comment.
These comments need updating too I think?
There was a problem hiding this comment.
I think the comments on this function all still apply. I'll take a look at whether any other changes need corresponding comments updated, though.
kripken
left a comment
There was a problem hiding this comment.
Might be worth measuring this on some code. If it is slow, we could do similar things like we do with pop here - only refresh this data if we generate new code that forces that, etc.
This entirely eliminates the class of bugs due to stale data in the
parentsandlocalGraphafter a previous allocation has been optimized.