Fix isGenerative on calls and test via improving OptimizeInstructions::areConsecutiveInputsEqual()#6481
Conversation
src/ir/properties.cpp
Outdated
| struct Scanner : public PostWalker<Scanner> { | ||
| bool generative = false; | ||
|
|
||
| void visitCall(Call* curr) { generative = true; } |
There was a problem hiding this comment.
Can we see if the callee has side effects here? If it doesn't, then it must be pure and we know it can't be generative. (Unless we're missing generativity itself as a side effect?)
There was a problem hiding this comment.
Good point that we could do better here, I'll add a TODO. Effects are not enough, though: imagine the body is a struct.new, that has no effects but it is generative. So we'd need to recursively check the generativity property.
| )) | ||
|
|
||
| ;; But generative effects in the fallthrough values themselves block us. | ||
| (drop (f64.abs |
There was a problem hiding this comment.
It would be nice to put each of these cases in its own function so that it appears adjacent to its output in the test file.
| (call $set-i32 | ||
| (i32.const 1337) | ||
| ) |
There was a problem hiding this comment.
Does the code do the right thing if this is a local.set $x so the "matching" tee and get are not matching after all?
There was a problem hiding this comment.
Good point, that was wrong. Fixed now and added testing.
2 participants
"Generative" is what we call something like a
struct.newthat may be syntacticallyidentical to another
struct.new, but each time a new value is generated. The sameis true for calls, which can do anything, including return a different value for
syntactically identical calls.
This was not a bug because the main user of
isGenerative,areConsecutiveInputsEqual(), was too weak to notice, that is, it gave up sooner,for other reasons. This PR improves that function to do a much better check,
which makes the fix necessary to prevent regressions.
This is not terribly important for itself, but will help a later PR that will add code
that depends more heavily on
areConsecutiveInputsEqual().