Vacation gardening #5920
Merged
Vacation gardening #5920
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
…Block. This eliminates all inline creation of SILBasicBlock via placement new. There are a few reasons to do this: 1. A SILBasicBlock is always created with a parent function. This commit formalizes this into the SILBasicBlock API by only allowing for SILFunctions to create SILBasicBlocks. This is implemented via the type system by making all SILBasicBlock constructors private. Since SILFunction is a friend of SILBasicBlock, SILFunction can still create a SILBasicBlock without issue. 2. Since all SILBasicBlocks will be created in only a few functions, it becomes very easy to determine using instruments the amount of memory being allocated for SILBasicBlocks by simply inverting the call tree in Allocations. With LTO+PGO, normal inlining can occur if profitable so there shouldn't be overhead that we care about in shipping compilers.
Before this commit all code relating to handling arguments in SILBasicBlock had somewhere in the name BB. This is redundant given that the class's name is already SILBasicBlock. This commit drops those names. Some examples: getBBArg() => getArgument() BBArgList => ArgumentList bbarg_begin() => args_begin()
…nt list manipulation section. I am not sure why it was put here originally, but I believe it is still there due to bitrot.
The BasicBlock suffix is redundant.
…eArgument instead of inline placement new. The reasoning here is the same as in e42bf07.
|
@swift-ci Please test and merge |
|
Linux failed b/c I needed to update lldb |
|
swift-lldb#82 @swift-ci Please test linux platform |
|
@swift-ci Please test linux platform |
|
Build failed |
|
Linux full test build is broken. I am going to try the smoke test... |
|
@swift-ci Please smoke test linux platform |
jjgod
added a commit
to jjgod/swift
that referenced
this pull request
Nov 25, 2016
[448/1413] Building CXX object lib/SILOptimizer/CMakeFiles/swiftSILOptimizer.dir/IPO/ClosureSpecializer.cpp.o
/Volumes/Home/swift-source/swift/lib/SILOptimizer/IPO/ClosureSpecializer.cpp:569:14: warning: unused variable 'M' [-Wunused-variable]
SILModule &M = Cloned->getModule();
^
1 warning generated.
Introduced by swiftlang#5920.
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.
This PR contains a few different gardening changes. The main meat of the change is:
This makes the API of SILArgument, SILBasicBlock follow the already enforced contract that a SILBasicBlock can only be created given its parent SILFunction and a SILArgument can only be created given its parent block.
As an additional benefit, it makes looking at memory allocations related to SILArguments and SILBasicBlocks significantly easier since when compiling without LTO all SILArguments/SILBasicBlocks are guaranteed to be allocated in the same call frame. This implies one only needs to perform a simple invert call tree in instruments to find all of such allocations.