call.without.effects is always possible for CSE #7568
Conversation
| // call.without.effects is always possible for CSE. | ||
| if (Intrinsics(*getModule()).isCallWithoutEffects(curr)) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Hmm, isn't the isGenerative check at the end of the function still important? For example, a call.without.effects might return a random number on each call - that has no effects, but we can't CSE it.
There was a problem hiding this comment.
I thought returning a random number is considered an effect. Meaning if a function return random number, it should not be called/marked as "call.without.effect".
But if I'm misunderstanding, should we then introduce "call.pure"? So that it can be applied to pure function call and take benefit from CSE.
There was a problem hiding this comment.
Effects are the things we consider in effects.h, see the properties there (like reading memory, throwing, etc.).
Something like generating random numbers, but having no other side effects, is covered under what we call "generativity" (generating unexpected/different values). That in theory could have been part of effects.h, but it is rare enough to matter that we left it separate.
So something like "pure" calls would be needed to optimize here. Actually I am planning to rework the intrinsics soon anyhow, avoiding the awkward import we use atm, in favor of something more like the Branch Hinting and Compilation Hints proposals. I am adding Branch Hinting now, so once the shared infrastructure is ready, we can move this to there. And I think we may want to rename it "pure" at that point, and include generativity - does that sound good?
There was a problem hiding this comment.
Thank you, including generativity in to "pure" annotation/hint sounds good.
It's indeed awkward to apply "call.without.effects" in the current way.. It might be a breaking change (since we currently use "call.without.effects" in the current way) but sound be easy enough to migrate.
For this PR, I think you can close it now if you want. If possible, please cc me in the issue/PR you planned for reworking the intrinsics.
There was a problem hiding this comment.
I opened #7574 - please provide feedback there if you have any.
LocalCSE has a isPossible check to see if an expression can be eliminated. Currently it treats all function calls as not possible. This PR proposes that it should consider "call.without.effects" function calls as always possible.
This will allow reusing result of a previously called without-effect function with the same arguments.