Skip to content

Improve one of the comments in COWArrayOpts. #29532

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 1 commit into from
Jan 29, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -849,11 +849,16 @@ bool COWArrayOpt::hoistMakeMutable(ArraySemanticsCall MakeMutable,

// Check whether we can hoist make_mutable based on the operations that are
// in the loop.
// Note that in this case we don't verify that the array buffer is not aliased
// and therefore we must be conservative if the make_mutable is executed
// conditionally (i.e. doesn't dominate all exit blocks).
// The test SILOptimizer/cowarray_opt.sil: dont_hoist_if_executed_conditionally
// shows the problem.
//
// Hoisting make_mutable releases the original array storage. If an alias of
// that storage is accessed on any path reachable from the loop header that
// doesn't already pass through the make_mutable, then hoisting is
// illegal. hasLoopOnlyDestructorSafeArrayOperations checks that the array
// storage is not accessed within the loop. However, this does not include
// paths exiting the loop. Rather than analyzing code outside the loop, simply
// check that the original make_mutable dominates all exits. The test
// SILOptimizer/cowarray_opt.sil: dont_hoist_if_executed_conditionally shows
// the problem.
if (hasLoopOnlyDestructorSafeArrayOperations() && dominatesExits) {
// Done. We can hoist the make_mutable.
// We still need the array uses later to check if we can add loads to
Expand Down