Update global init optimization to work with vars #29122
Conversation
|
I'm sorry, I forgot about this. Do you want to fix the conflict and I'll kick off PR testing? |
|
Don't worry about it. I'll update the conflict tomorrow morning. |
|
@zoecarver I think once you merge another PR or two, you can apply for commit access. It's somewhat misleadingly named because it allows you to kick off CI testing yourself. (We'd still want a review from someone before merging) Details here: https://swift.org/contributing/#contributing-code = Commit Access |
|
@slavapestov thank you! I'll look into that. Might be a few more PRs, though, I'm only at two unmodified PRs as of now. |
|
Don't trigger the bots yet. I just ran the test suit and discovered I need to fix something. |
| // is only one use, it must be the use that we are trying to optimize, so | ||
| // that is OK. If there is more than one use, one of the other uses may | ||
| // have a store attached to it which means there may be more than one | ||
| // assignment, so return false. |
There was a problem hiding this comment.
I added this additional check to prevent an edge case where global uses were improperly optimized away. In the future, GlobalInitCallMap applies could theoretically be traced to a store instruction that could be added to GlobalVarStore which would be beneficial for a few reasons (including that we could get rid of this extra check and optimize more code correctly).
|
@slavapestov this should be ready for testing now. |
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
|
@slavapestov can you please merge this for me now that the tests passed? |
|
@slavapestov thank you again :) |
This patch updates
isAssignedOnlyOnceInInitializerto return weather a var can be used externally instead of just returning false.If there are multiple stores to the var, it will be added to
GlobalVarSkipProcessingso, it will return false.