Skip to content

[MoveOnlyAddressChecker] Fix representation for initialized fields. #66955

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

Merged
merged 6 commits into from
Jun 28, 2023

Conversation

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented Jun 27, 2023

Based on #66945 , only the last two commits are new here.

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 #66728 , a SmallBitVector is the representation chosen.

rdar://111391893

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@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.

LGTM Thanks!

@nate-chandler
Copy link
Contributor Author

CC: @kavon @gottesmm @jckarter

Used the TypeTreeLeafTypeRange::setBits helper rather than looping over
the range and setting the bits in place.
In preparation to share the getOrCreateConsumingBlock functionality with
another overload of recordConsumingBlock.
@nate-chandler
Copy link
Contributor Author

Rebased on top of main to avoid conflict with llvm:: prefixing of Optional. (The previous force push to make that change locally wasn't enough.)

@@ -621,11 +624,11 @@ struct UseState {

/// A map from an instruction that initializes memory to the description of
/// the part of the type tree that it initializes.
llvm::SmallMapVector<SILInstruction *, TypeTreeLeafTypeRange, 4> initInsts;
llvm::SmallMapVector<SILInstruction *, SmallBitVector, 4> initInsts;
Copy link
Contributor

Choose a reason for hiding this comment

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

@nate-chandler shouldn't this also use inst to bit map?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

The new overload iterates over the contiguous ranges in the bit vector
and calls through to the overload that takes a range.
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
swiftlang#66728 , a SmallBitVector is the
representation chosen.

rdar://111356251
In preparation for having a third instance getting or creating the bits
affected by an instruction, introduce a typealias for maps
SILInstruction->SmallBitVector and a helper function that returns
a reference to the SmallBitVector for an instruction and two others that
set into those bits the bits from another bit vector or from a range.
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
swiftlang#66728 , a SmallBitVector is the
representation chosen.

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

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please smoke test macos platform

@nate-chandler nate-chandler merged commit 06f53fe into swiftlang:main Jun 28, 2023
@nate-chandler nate-chandler deleted the rdar111391893 branch June 28, 2023 03:55
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.

3 participants