Conversation
|
The refactoring in OptimizeInstructions had an additional benefit, it turns out. I moved that out to #8355 |
|
Btw, I noticed that C has something similar to this: |
| // If the called function is idempotent, then it does not generate new | ||
| // values on each call. |
There was a problem hiding this comment.
I don't think this is correct. An idempotent function could return new values on each call as long as it gets different parameters each time (for example if it's either creating new values or looking them up in a cache). It seems like this would cause problems for users of isShallowlyGenerative, at least.
There was a problem hiding this comment.
I think that's already in the definition of generative?
Lines 558 to 559 in 4efcdfe
If the inputs change, all bets are off.
Users of isShallowlyGenerative need to look at children, correct. LocalCSE does that, basically the "shallow" version is an incremental one, more efficient sometimes, but the children must be scanned as well.
| // No effects are possible. | ||
| rightMightHaveEffects = false; |
There was a problem hiding this comment.
I would also expect us to have to check that the child values match here. Do we not?
There was a problem hiding this comment.
We check that the entire expressions match, including children, on line 2891.
It is just that it seems more efficient to do it in this order (the equality check scans both inputs, not just one, so early exit can avoid scanning half the data).
| // We may want to move the function-level annotations to a dedicated | ||
| // field outside the map. |
There was a problem hiding this comment.
Yeah, I think this would be a nice cleanup anyway.
| ;; CHECK: (func $potent (type $0) (param $x f32) (result f32) | ||
| ;; CHECK-NEXT: (call $import) | ||
| ;; CHECK-NEXT: ) | ||
| (func $potent (param $x f32) (result f32) |
There was a problem hiding this comment.
Or "alipotent" from the Latin root "Ali-" meaning "different" 😀
| ) | ||
| ) | ||
| ) | ||
| ;; Here we succeed, as the calls are marked idempotent. |
There was a problem hiding this comment.
Let's add cases where just one or the other of the calls is marked idempotent. (Although what would the meaning of that be? Optimizable whenever the second call is marked idempotent, perhaps?)
There was a problem hiding this comment.
Yes, when the second is marked, that seems right. Added.
There was a problem hiding this comment.
Is this enough to write CSE tests that show idempotent function calls being deduplicated?
There was a problem hiding this comment.
CSE needs to actually find the two calls, and only then can it infer that the second is removable. So this does require logic in each pass, like the areEqualAndFoldable part here. That and OnceReduction I can do later.
An idempotent function may do work when called, but if called again with
the same parameters, it does no work and it returns the same value (if it
returns a value).
This uses the new intrinsic in OptimizeInstructions. More things are
possible too.
See discussion in #7574