SIL: replace the notifyDeleteHandlers mechanism with delayed instruction deletion #37657
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When an instruction is "deleted" from the SIL, it is put into the SILModule::scheduledForDeletion list. The instructions in this list are eventually deleted for real in SILModule::flushDeletedInsts(), which is called by the pass manager after each pass run. In other words: instruction deletion is deferred to the end of a pass. This avoids dangling instruction pointers within the run of a pass and in analysis caches. Note that the analysis invalidation mechanism ensures that analysis caches are invalidated before flushDeletedInsts().
…e cache keys
Instead of caching alias results globally for the module, make AliasAnalysis a FunctionAnalysisBase which caches the alias results per function.
Why?
* So far the result caches could only grow. They were reset when they reached a certain size. This was not ideal. Now, they are invalidated whenever the function changes.
* It was not possible to actually invalidate an alias analysis result. This is required, for example in TempRValueOpt and TempLValueOpt (so far it was done manually with invalidateInstruction).
* Type based alias analysis results were also cached for the whole module, while it is actually dependent on the function, because it depends on the function's resilience expansion. This was a potential bug.
I also added a new PassManager API to directly get a function-base analysis:
getAnalysis(SILFunction *f)
The second change of this commit is the removal of the instruction-index indirection for the cache keys. Now the cache keys directly work on instruction pointers instead of instruction indices. This reduces the number of hash table lookups for a cache lookup from 3 to 1.
This indirection was needed to avoid dangling instruction pointers in the cache keys. But this is not needed anymore, because of the new delayed instruction deletion mechanism.
It's not needed anymore
It's not needed anymore with delayed instruction deletion. It was used for two purposes: 1. For analysis, which cache instructions, to avoid dangling instruction pointers 2. For passes, which maintain worklists of instructions, to remove a deleted instructions from the worklist. This is now done by checking SILInstruction::isDeleted().
|
@swift-ci test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The notifyDeleteHandlers were introduced long time ago mainly to avoid dangling instruction pointer in analysis caches.
With delaying the deletion of instructions after a run function of a pass has finished, the notification mechanism is not needed anymore.
This is another step of simplifying SIL and the optimizer. The old invalidation mechanism was especially tricky in AliasAnalysis. Now it's much simpler with reducing the number of hash table lookups from 3 to 1 for a cache lookup.
This change required to make the AliasAnalysis a function-level analysis, which has other benefits, too.
For details see the commit messages.