[NFC] SIL: Clarified deinit barrier APIs. #70774
Merged
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 `mayReadOrWriteMemory`, the `mayAccessPointer` predicate relies on the `visitAccessedAddress` utility to determine whether an instruction may access a pointer. That utility doesn't (and can't, without changing the representation of the builtin) visit `RawPointer` operands as used by memmove/memcpy builtins. Previously, this resulted `mayAccessPointer` returning `false` for such builtins. Here, this is fixed by making the predicate handle `BuiltinInsts` separately. Instead of just always returning `true` in the face of a builtin, rely on the BuiltinInfo associated with a BuiltinInst and use `mayReadOrWriteMemory`. rdar://120656227
Dropped the NotConsideringSideEffects suffix.
atrick
approved these changes
Jan 8, 2024
nate-chandler
force-pushed
the
nfc/20240108/1/deinit-barrier-component-predicates
branch
from
c47e7d2 to
30cacdf
Compare
The three predicates are all now defined and defined correctly on builtins to return true whenever mayReadOrWriteMemory is true.
An instruction is a deinit barrier whenever one of three component predicates is true for it. In the case of applies, it is true whenever one of those three predicates is true for any of the instructions in any of its callees; that fact is cached in the side-effect analysis of every function. If side-effect analysis or callee analysis is unavailable, in order to define each of those three component predicates on a `FullApplySite`/`EndApplyInst`/`AbortApplyInst`, it would be necessary to define them to conservatively return true: it isn't known whether any of the instructions in any of the callees were deinit barriers. Refactored the two versions of the deinit barrier predicate (namely `Instruction.isDeinitBarrier(_:) and `swift::mayBeDeinitBarrierNotConsideringSideEffects`) to handle `FullApplySite`/`EndApplyInst`/`AbortApplyInst`s specially first (to look up the callees' side-effect and to conservatively bail, respectively). Asserted that the three component predicates are not called with `FullApplySite`/`EndApplyInst`/`AbortApplyInst`s. Callers should instead use the `isDeinitBarrier` APIs. An alternative would be to conservatively return true from the three components. That seems more likely to result in direct calls to these member predicates, however, and at the moment at least there is no reason for such calls to exist. If some other caller besides the deinit-barrier predicates needs to call this function, side-effect analysis should be updated to cache these three properties separately at that point.
nate-chandler
force-pushed
the
nfc/20240108/1/deinit-barrier-component-predicates
branch
from
30cacdf to
82b7495
Compare
|
@swift-ci please test |
nate-chandler
deleted the
nfc/20240108/1/deinit-barrier-component-predicates
branch
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 adds three commits on top of #70773 .
An instruction is a deinit barrier whenever one of three component predicates is true for it. In the case of applies, it is true whenever one of those three predicates is true for any of the instructions in any of its callees; that fact is cached in the side-effect analysis of every function.
If side-effect analysis or callee analysis is unavailable, in order to define each of those three component predicates on a
FullApplySite/EndApplyInst/AbortApplyInst, it would be necessary to define them to conservatively return true: it isn't known whether any of the instructions in any of the callees were deinit barriers.Refactored the two versions of the deinit barrier predicate (namely
Instruction.isDeinitBarrier(_:)andswift::mayBeDeinitBarrierNotConsideringSideEffects) to handleFullApplySite/EndApplyInst/AbortApplyInsts specially first (to look up the callees' side-effect and to conservatively bail, respectively). Asserted that the three component predicates are not called withFullApplySite/EndApplyInst/AbortApplyInsts. Callers should instead use theisDeinitBarrierAPIs.An alternative would be to conservatively return true from the three components. That seems more likely to result in direct calls to these member predicates, however, and at the moment at least there is no reason for such calls to exist. If some other caller besides the deinit-barrier predicates needs to call this function, side-effect analysis should be updated to cache these three properties separately at that point.