From 17d9d20275aac858104e92b3d33e0b5bdcfe0344 Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Wed, 29 Jan 2020 10:23:18 -0800 Subject: [PATCH] Improve one of the comments in COWArrayOpts. --- lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp b/lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp index c48c161cb28ed..b4265a2690aa3 100644 --- a/lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp +++ b/lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp @@ -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