ConstraintAnalysis: Track relevant locals#8921
Conversation
| // to is relevant as well. We store pairs here of key=source, value=targets. | ||
| std::unordered_map<Index, std::vector<Index>> localCopyTargets; | ||
|
|
||
| void markRelevant(Expression* curr) { |
There was a problem hiding this comment.
| void markRelevant(Expression* curr) { | |
| void maybeMarkRelevant(Expression* curr) { |
Since this does not unconditionally mark the expression relevant.
| // Track local copies too, as if one local is relevant, another it is copied | ||
| // to is relevant as well. We store pairs here of key=source, value=targets. | ||
| std::unordered_map<Index, std::vector<Index>> localCopyTargets; |
There was a problem hiding this comment.
Shouldn't we be propagating relevance backward from copy destination to copy source? If I have a local $x whose value is never set or compared to anything, then it is copied to relevant $y, then I want to have that initial $x = 0 constraint. In contrast, if I have relevant $y and it is copied to irrelevant $z, then there is no need to consider $z relevant.
There was a problem hiding this comment.
Shouldn't we be propagating relevance backward from copy destination to copy source?
Good catch! Yes, the default value is tricky that way. Fixed.
In contrast, if I have relevant $y and it is copied to irrelevant $z, then there is no need to consider $z relevant.
Hmm, I thought that was true due to contradictions, but you're right, I can't make a testcase that shows the problem, good catch as well. Fixed.
| // happen, but intermediate optimizations can make things become relevant, | ||
| // consider this: | ||
| // | ||
| // x == (y < 10) |
There was a problem hiding this comment.
Isn't this change not NFC because of this? IIUC, we're losing a very small amount of optimization power here.
There was a problem hiding this comment.
Yes, fair enough, I'll remove the NFC. This makes us 20% faster in return for needing another cycle in rare cases.
Co-authored-by: Thomas Lively <tlively123@gmail.com>
kripken
changed the title
2 participants
By avoiding work on irrelevant locals - ones we can't prove anything for -
we make the pass 20% faster.