SIL: add mark_dependence_addr #80263
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
atrick
requested review from
eeckstein,
jckarter,
xymus,
kavon and
rjmccall
as code owners
|
@swift-ci test |
|
@swift-ci benchmark |
|
@swift-ci test source compatibility |
|
@swift-ci smoke test |
eeckstein
reviewed
Mar 26, 2025
| UnsafePointer) or pointer escape (unknown use). | ||
| The result is the forwarded value of `%value`. If `%value` is an | ||
| address, then result is also an address, and the semantics are the | ||
| same as the non-address form: the dependency is on any value derived |
There was a problem hiding this comment.
I feel we need a definition what "derived" means.
In your example you say that a loaded value is a derived value. But what e.g. if the loaded value is stored again to another memory location. Is this "derived" as well? What about copied values? Etc.
| mark_dependence_addr [nonescaping] %address : $*T on %base : $Builtin.NativeObject | ||
| ``` | ||
|
|
||
| The in-memory value at `%address` depends on the value of `%base`. |
There was a problem hiding this comment.
What's the difference to mark_dependence with an address operand?
There was a problem hiding this comment.
Last time we discussed this, there wasn't any confusion. The documentation is as precise as I can make it. sounds like we just need to revise the wording or the examples as a follow up.
|
source-compat ObjectMapper is failing on main. |
beccadax
added a commit
to beccadax/swift
that referenced
this pull request
Apr 5, 2025
When a C enum has multiple constants with the same value, ClangImporter selects one of them to import as an `EnumElementDecl` and imports the others as `VarDecl` “aliases” of that one. This helps preserve the invariant that each case of an enum has a unique raw value and is distinct for exhaustiveness checking. However, a bug in that logic could sometimes cause the canonical case to be imported *twice*—once as an `EnumElementDecl` and again as a `VarDecl` alias. In this situation, the `EnumElementDecl` was not added to the enum’s member list, resulting in a malformed AST. This bug has apparently been present since early 2017 (!), but it seems to have been harmless until recently, when the `ComputeSideEffects` SIL pass recently became sensitive to it (probably because of either swiftlang#79872 or swiftlang#80263). Correct this problem by modifying the memoization logic to retrieve the canonical case’s clang-side constant so the subsequent check will succeed. Additionally change a variable name and add comments to help clarify this code for future maintainers. In lieu of adding new unit tests, this commit adds a (slightly expensive) conditional assertion to catch this kind of AST malformation. There are actually about twenty tests that will fail with just the assertion and not the fix, but I’ve updated one of them to enable the assertion even in release mode. Fixes rdar://148213237. Followup to swiftlang#80487, which added related assertions to the SIL layer.
beccadax
added a commit
to beccadax/swift
that referenced
this pull request
Apr 11, 2025
When a C enum has multiple constants with the same value, ClangImporter selects one of them to import as an `EnumElementDecl` and imports the others as `VarDecl` “aliases” of that one. This helps preserve the invariant that each case of an enum has a unique raw value and is distinct for exhaustiveness checking. However, a bug in that logic could sometimes cause the canonical case to be imported *twice*—once as an `EnumElementDecl` and again as a `VarDecl` alias. In this situation, the `EnumElementDecl` was not added to the enum’s member list, resulting in a malformed AST. This bug has apparently been present since early 2017 (!), but it seems to have been harmless until recently, when the `ComputeSideEffects` SIL pass recently became sensitive to it (probably because of either swiftlang#79872 or swiftlang#80263). Correct this problem by modifying the memoization logic to retrieve the canonical case’s clang-side constant so the subsequent check will succeed. Additionally change a variable name and add comments to help clarify this code for future maintainers. In lieu of adding new unit tests, this commit adds a (slightly expensive) conditional assertion to catch this kind of AST malformation. There are actually about twenty tests that will fail with just the assertion and not the fix, but I’ve updated one of them to enable the assertion even in release mode. Fixes rdar://148213237. Followup to swiftlang#80487, which added related assertions to the SIL layer.
beccadax
added a commit
to beccadax/swift
that referenced
this pull request
Apr 15, 2025
When a C enum has multiple constants with the same value, ClangImporter selects one of them to import as an `EnumElementDecl` and imports the others as `VarDecl` “aliases” of that one. This helps preserve the invariant that each case of an enum has a unique raw value and is distinct for exhaustiveness checking. However, a bug in that logic could sometimes cause the canonical case to be imported *twice*—once as an `EnumElementDecl` and again as a `VarDecl` alias. In this situation, the `EnumElementDecl` was not added to the enum’s member list, resulting in a malformed AST. This bug has apparently been present since early 2017 (!), but it seems to have been harmless until recently, when the `ComputeSideEffects` SIL pass recently became sensitive to it (probably because of either swiftlang#79872 or swiftlang#80263). Correct this problem by modifying the memoization logic to retrieve the canonical case’s clang-side constant so the subsequent check will succeed. Additionally change a variable name and add comments to help clarify this code for future maintainers. In lieu of adding new unit tests, this commit adds a (slightly expensive) conditional assertion to catch this kind of AST malformation. There are actually about twenty tests that will fail with just the assertion and not the fix, but I’ve updated one of them to enable the assertion even in release mode. Fixes rdar://148213237. Followup to swiftlang#80487, which added related assertions to the SIL layer.
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.
SIL: add mark_dependence_addr
And support mark_dependence_addr in SIL passes.
Necessary whenever an addressable value depends on another value. We will need dataflow to determine the lifetime of the base.