Skip to content
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

[MoveOnlyAddressChecker] Fix representation for used fields. #66728

Merged
merged 1 commit into from
Jun 18, 2023

Conversation

nate-chandler
Copy link
Contributor

The address checker records uses in its livenessUses map. Previously, that map mapped from an instruction to a range of fields of the type. But an instruction can use multiple discontiguous fields of a single value. Here, such instructions are properly recorded by fixing the map to store a bit vector for each instruction.

rdar://110676577

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

Copy link
Member

@atrick atrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! I reviewed for CCC.

I just commented in a few places that we prefer to iterate over the indices in a range rather than allocating and initializing a new bitset and rather than iterating over a bitset.

include/swift/SIL/FieldSensitivePrunedLiveness.h Outdated Show resolved Hide resolved
lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp Outdated Show resolved Hide resolved
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

The address checker records uses in its livenessUses map.  Previously,
that map mapped from an instruction to a range of fields of the type.
But an instruction can use multiple discontiguous fields of a single
value.  Here, such instructions are properly recorded by fixing the map
to store a bit vector for each instruction.

rdar://110676577
@nate-chandler
Copy link
Contributor Author

@swift-ci please test and merge

@swift-ci swift-ci merged commit bbed05b into apple:main Jun 18, 2023
5 of 6 checks passed
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that reinit fields in its
reinitInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can use multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was reinit'd by an already reinit'ing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be reinit'd by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111356251
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that reinit fields in its
reinitInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can use multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was reinit'd by an already reinit'ing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be reinit'd by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111356251
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that initialize fields in its
initInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can initialize multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was initialized by an already initializing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be initialized by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111391893
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that reinit fields in its
reinitInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can use multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was reinit'd by an already reinit'ing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be reinit'd by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111356251
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that initialize fields in its
initInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can initialize multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was initialized by an already initializing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be initialized by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111391893
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that reinit fields in its
reinitInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can use multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was reinit'd by an already reinit'ing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be reinit'd by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111356251
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that initialize fields in its
initInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can initialize multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was initialized by an already initializing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be initialized by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111391893
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that reinit fields in its
reinitInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can use multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was reinit'd by an already reinit'ing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be reinit'd by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111356251
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that initialize fields in its
initInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can initialize multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was initialized by an already initializing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be initialized by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111391893
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that initialize fields in its
initInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can initialize multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was initialized by an already initializing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be initialized by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111391893
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that reinit fields in its
reinitInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can use multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was reinit'd by an already reinit'ing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be reinit'd by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111356251
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that initialize fields in its
initInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can initialize multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was initialized by an already initializing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be initialized by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111391893
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that reinit fields in its
reinitInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can use multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was reinit'd by an already reinit'ing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be reinit'd by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111356251
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 27, 2023
The address checker records instructions that initialize fields in its
initInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can initialize multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was initialized by an already initializing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be initialized by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111391893
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Jun 28, 2023
The address checker records instructions that initialize fields in its
initInsts map.  Previously, that map mapped from an instruction to a
range of fields of the type.  But an instruction can initialize multiple
discontiguous fields of a single value.  (Indeed an attempt to add a
second range that was initialized by an already initializing
instruction--even if it were overlapping or adjacent--would have no
effect and the map wouldn't be updated.)  Here, this is fixed by fixing
the representation and updating the storage whenver a new range is seen
to be initialized by the instruction.  As in
apple#66728 , a SmallBitVector is the
representation chosen.

rdar://111391893
@nate-chandler nate-chandler deleted the rdar110676577_2 branch July 5, 2023 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants