Do not merge functions differing in semantics-altering annotations#8361
Do not merge functions differing in semantics-altering annotations#8361kripken merged 43 commits intoWebAssembly:mainfrom
Conversation
src/ir/function-utils.h
Outdated
| if (leftAnnotations.jsCalled != rightAnnotations.jsCalled || | ||
| leftAnnotations.removableIfUnused != rightAnnotations.removableIfUnused) { |
There was a problem hiding this comment.
Rather than checking inequality of the semantics-affecting annotations, can we instead check equality of the known-non-functional annotations? Then when we add new annotations, they will be conservatively prevent merging.
There was a problem hiding this comment.
OTOH doing the current "unsafe" thing allows the fuzzer tell us when we forgot to update this.
There was a problem hiding this comment.
Yeah, this is tricky. The fuzzer doesn't normally run on this code - fuzzing semantic-altering hints is hard, look at the work it took to fuzz js.called - but there is at least a chance of it telling us of mistakes. While erroring the other way means not merging code that could be, and losing on code size, silently... not sure what's best.
I did refactor wasm.h now to add functions with explicit comparison of semantics changing/preserving, so when adding a new one, you have to at least decide on that, in the most central place. wdyt?
2 participants
If function A is
js.calledand B is not, do not merge them, as the resultwould have to either have the annotation or not, and neither is necessarily
what the user wanted.