-
Notifications
You must be signed in to change notification settings - Fork 10.5k
SIL optimizations: Implement the new API for analysis invalidation. #8083
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SIL optimizations: Implement the new API for analysis invalidation. #8083
Conversation
@swift-ci Please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's great to see this API cleaned up. There's one if-condition that looks like it needs to be cleaned up. And I asked for more explanation in some places where we skip invalidation. Otherwise fantastic.
} | ||
|
||
virtual void invalidate(SILFunction *F, InvalidationKind K) { invalidate(K); } | ||
/// Invalidate all of the information for a specific function. | ||
virtual void invalidate(SILFunction *F, InvalidationKind K) override { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment: explain why this does nothing.
virtual void invalidate(SILFunction *F, InvalidationKind K) override { } | ||
|
||
/// Notify the analysis about a newly created function. | ||
virtual void notifyAddFunction(SILFunction *F) override { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment: explain why this does nothing.
|
||
/// Notify the analysis about a function which will be deleted from the | ||
/// module. | ||
virtual void notifyDeleteFunction(SILFunction *F) override { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment: explain why this does nothing.
@@ -34,6 +34,22 @@ class DestructorAnalysis : public SILAnalysis { | |||
/// Returns true if destruction of T may store to memory. | |||
bool mayStoreToMemoryOnDestruction(SILType T); | |||
|
|||
/// No invalidation is needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment: explain why no invalidation is needed.
@@ -983,17 +971,35 @@ class CapturePromotionPass : public SILModuleTransform { | |||
processFunction(&F, Worklist); | |||
|
|||
if (!Worklist.empty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this if-condition for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. It should be removed
Get rid of the utility class ClosureSpecializer. NFC.
There are now separate functions for function addition and deletion instead of InvalidationKind::Function. Also, there is a new function for witness/vtable invalidations. rdar://problem/29311657
…SignatureOpts and GenericSpecializer anymore. Because now the BasicCalleeAnalysis is not invalidated anyway by these optimizations. It’s only invalidated by DeadFunctionElimination.
Thanks @atrick for reviewing!
de928a3
to
0e068ea
Compare
@swift-ci Please smoke test and merge |
1 similar comment
@swift-ci Please smoke test and merge |
There are now separate functions for function addition and deletion instead of InvalidationKind::Function.
Also, there is a new function for witness/vtable invalidations.
rdar://problem/29311657
Also:
No need to preserve BasicCalleeAnalysis from invalidation in FunctionSignatureOpts and GenericSpecializer anymore.
Because now the BasicCalleeAnalysis is not invalidated anyway by these optimizations.
It’s only invalidated by DeadFunctionElimination.