[ownership] Define a new instruction copy_unmanaged_value. #26839
+223
−107
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
This provides a singular instruction for convert an unmanaged value to a ref, then strong_retain it. I expanded the definition of UNCHECKED_REF_STORAGE to include these copy like instructions. This instruction is valid in all SIL. The reason why I am adding this instruction is that currently when we emit an access to an unowned (unsafe) ivar, we use an unmanaged_to_ref and a strong retain. This can look to the optimizer like a strong retain that can potentially be optimized. By combining the two together into a new instruction, we can avoid this potential problem since the pattern matching will break.
gottesmm
commented
Aug 26, 2019
| case SILInstructionKind::Name##RetainInst: \ | ||
| case SILInstructionKind::Name##ReleaseInst: \ | ||
| case SILInstructionKind::StrongRetain##Name##Inst: \ | ||
| case SILInstructionKind::Copy##Name##ValueInst: |
There was a problem hiding this comment.
NOTE: I centralized this Copy##Name##ValueInst into COMMON_ALWAYS_OR_SOMETIMES_LOADABLE_CHECKED_REF_STORAGE above.
| @@ -157,7 +160,6 @@ static bool isRLEInertInstruction(SILInstruction *Inst) { | |||
| case SILInstructionKind::IsEscapingClosureInst: | |||
| case SILInstructionKind::IsUniqueInst: | |||
| case SILInstructionKind::FixLifetimeInst: | |||
| case SILInstructionKind::CopyUnownedValueInst: | |||
There was a problem hiding this comment.
I just moved this slightly further up.
| @@ -154,6 +154,9 @@ class MemoryBehaviorVisitor | |||
| } | |||
| REFCOUNTINC_MEMBEHAVIOR_INST(StrongRetainInst) | |||
| REFCOUNTINC_MEMBEHAVIOR_INST(RetainValueInst) | |||
| #define UNCHECKED_REF_STORAGE(Name, ...) \ | |||
| REFCOUNTINC_MEMBEHAVIOR_INST(Name##RetainValueInst) \ | |||
There was a problem hiding this comment.
I added this b/c these are manual retain, release instructions and should be treated as ref counting membehavior insts.
| @@ -2083,40 +2083,41 @@ class SILVerifier : public SILVerifierBase<SILVerifier> { | |||
| "Operand of " #name "_to_ref does not have the " \ | |||
| "operand's type as its referent type"); \ | |||
| } | |||
| #define ALWAYS_LOADABLE_CHECKED_REF_STORAGE_HELPER(Name, name, closure) \ | |||
There was a problem hiding this comment.
I am not sure why clang-format reformatted this. I can undo this hunk if you want.
| case SILInstructionKind::Name##RetainInst: \ | ||
| case SILInstructionKind::Name##ReleaseInst: \ | ||
| case SILInstructionKind::StrongRetain##Name##Inst: | ||
| #define UNCHECKED_REF_STORAGE(Name, ...) \ |
There was a problem hiding this comment.
I moved the Unmanaged*ValueInst under UNCHECKED_REF_STORAGE (since Unmanaged is an unchecked ref storage).
|
@swift-ci test |
|
@lorentey I have a second PR that once this is in, just enables SILGen. So this is the meat. |
eeckstein
requested changes
Aug 26, 2019
|
@eeckstein I am going to merge this and do that in a follow on PR. |
gottesmm
added a commit
to gottesmm/swift
that referenced
this pull request
Aug 26, 2019
This commit adds docs for copy_unmanaged_value to SIL.rst as requested by @eeckstein in swiftlang#26839. I noticed while doing the PR that copy_unowned_value was also not documented, so I added docs for it as well as a bonus. = ).
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 provides a singular instruction for convert an unmanaged value to a ref,
then strong_retain it. I expanded the definition of UNCHECKED_REF_STORAGE to
include these copy like instructions. This instruction is valid in all SIL.
The reason why I am adding this instruction is that currently when we emit an
access to an unowned (unsafe) ivar, we use an unmanaged_to_ref and a strong
retain. This can look to the optimizer like a strong retain that can potentially
be optimized. By combining the two together into a new instruction, we can avoid
this potential problem since the pattern matching will break.