[dart2js] Fix load elimination alias bug Phis cause aliasing. Rename nonEscapingReceivers to unaliasedAllocations to make it clear that aliasing is what is important. escaping is just one way of creating aliases. Bug: #63243 Change-Id: I9cda5ac3d188b9869eed1b5ae271a118d4c07bfd Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/523200 Reviewed-by: Nate Biggs <natebiggs@google.com> Commit-Queue: Stephen Adams <sra@google.com>
diff --git a/pkg/compiler/lib/src/ssa/optimize.dart b/pkg/compiler/lib/src/ssa/optimize.dart index aae3fd2..04eaf24 100644 --- a/pkg/compiler/lib/src/ssa/optimize.dart +++ b/pkg/compiler/lib/src/ssa/optimize.dart
@@ -4505,6 +4505,15 @@ } memories[node.id] = memorySet; + // Visit phis to ensure their inputs are marked as aliased. Since phis + // are stored separately from the instruction list, they must be visited + // explicitly so that `killAffectedBy` is called on them. + HPhi? phi = node.phis.firstPhi; + while (phi != null) { + final next = phi.nextPhi; + phi.accept(this); + phi = next; + } HInstruction? instruction = node.first; while (instruction != null) { final next = instruction.next; @@ -4788,9 +4797,8 @@ /// Maps a receiver to a map of keys to value. final Map<HInstruction, Map<HInstruction, HInstruction?>> keyedValues = {}; - /// Set of objects that we know don't escape (or have not yet escaped) the - /// current function. - final Setlet<HInstruction> nonEscapingReceivers = Setlet(); + /// Set of allocations that are currently unaliased (and thus local to the function). + final Setlet<HInstruction> unaliasedAllocations = Setlet(); MemorySet(this.closedWorld); @@ -4806,8 +4814,8 @@ bool mayAlias(HInstruction? first, HInstruction? second) { if (mustAlias(first, second)) return true; if (isConcrete(first) && isConcrete(second)) return false; - if (nonEscapingReceivers.contains(first)) return false; - if (nonEscapingReceivers.contains(second)) return false; + if (unaliasedAllocations.contains(first)) return false; + if (unaliasedAllocations.contains(second)) return false; // Typed arrays of different types might have a shared buffer. if (couldBeTypedArray(first!) && couldBeTypedArray(second!)) return true; return _abstractValueDomain @@ -4831,10 +4839,10 @@ .isPotentiallyTrue; } - /// Returns whether [receiver] escapes the current function. - bool escapes(HInstruction? receiver) { + /// Returns whether [receiver] is aliased (i.e. not unaliased). + bool isAliased(HInstruction? receiver) { assert(receiver == null || receiver == receiver.nonCheck()); - return !nonEscapingReceivers.contains(receiver); + return !unaliasedAllocations.contains(receiver); } /// Kills locations that are imprecise due to many possible edges from @@ -4863,7 +4871,7 @@ void registerAllocation(HInstruction instruction) { assert(instruction == instruction.nonCheck()); - nonEscapingReceivers.add(instruction); + unaliasedAllocations.add(instruction); } /// Sets the [field] on [receiver] to contain [value]. Kills all potential @@ -4883,8 +4891,8 @@ return false; // TODO(14955): Remove this restriction? } // [value] is being set in some place in memory, we remove it from the - // non-escaping set. - nonEscapingReceivers.remove(value.nonCheck()); + // unaliased set. + unaliasedAllocations.remove(value.nonCheck()); final map = fieldValues.putIfAbsent(field, () => {}); bool isRedundant = map[receiver] == value; map.forEach((key, value) { @@ -4928,11 +4936,11 @@ /// the set of non-escaping objects in case [instruction] has non-escaping /// objects in its inputs. void killAffectedBy(HInstruction instruction) { - // Even if [instruction] does not have side effects, it may use non-escaping + // Even if [instruction] does not have side effects, it may use unaliased // objects and store them in a new object, which make these objects - // escaping. + // aliased. for (var input in instruction.inputs) { - nonEscapingReceivers.remove(input.nonCheck()); + unaliasedAllocations.remove(input.nonCheck()); } if (instruction.sideEffects.changesInstanceProperty() || @@ -4945,7 +4953,7 @@ fieldValues.forEach((Object element, map) { if (isFinal(element)) return; map.forEach((receiver, value) { - if (escapes(receiver)) { + if (isAliased(receiver)) { receiversToRemove.add(receiver); } }); @@ -4962,7 +4970,7 @@ if (instruction.sideEffects.changesIndex()) { keyedValues.forEach((receiver, map) { - if (escapes(receiver)) { + if (isAliased(receiver)) { map.forEach((index, value) { map[index] = null; }); @@ -4995,7 +5003,7 @@ HInstruction index, HInstruction value, ) { - nonEscapingReceivers.remove(value.nonCheck()); + unaliasedAllocations.remove(value.nonCheck()); keyedValues.forEach((key, values) { if (mayAlias(receiver, key)) { // Typed arrays that are views of the same buffer may have different @@ -5090,13 +5098,13 @@ MemorySet result = MemorySet(closedWorld); if (other == null) { // This is the first visit to a loop header ([other] is `null` because we - // have not visited the back edge). Copy the nonEscapingReceivers that are - // guaranteed to survive the loop because they are not escaped before - // method exit. + // have not visited the back edge). Copy the unaliasedAllocations that are + // guaranteed to survive the loop because they do not escape or become aliased + // before method exit. // TODO(sra): We should do a proper dataflow to find the maximal - // nonEscapingReceivers (a variant of Available-Expressions), which must + // unaliasedAllocations (a variant of Available-Expressions), which must // converge before we edit the program in [findCommonInstruction]. - for (HInstruction instruction in nonEscapingReceivers) { + for (HInstruction instruction in unaliasedAllocations) { bool isNonEscapingUse(HInstruction use) { if (use is HReturn) return true; // Escapes, but so does control. if (use is HFieldGet) return true; @@ -5130,7 +5138,7 @@ } if (instruction.usedBy.every(isNonEscapingUse)) { - result.nonEscapingReceivers.add(instruction); + result.unaliasedAllocations.add(instruction); } } return result; @@ -5168,9 +5176,9 @@ }); }); - for (var receiver in nonEscapingReceivers) { - if (other.nonEscapingReceivers.contains(receiver)) { - result.nonEscapingReceivers.add(receiver); + for (var receiver in unaliasedAllocations) { + if (other.unaliasedAllocations.contains(receiver)) { + result.unaliasedAllocations.add(receiver); } } return result; @@ -5188,7 +5196,7 @@ result.keyedValues[receiver] = Map.of(values); }); - result.nonEscapingReceivers.addAll(nonEscapingReceivers); + result.unaliasedAllocations.addAll(unaliasedAllocations); return result; } @@ -5221,8 +5229,8 @@ } }); - result.nonEscapingReceivers.addAll( - nonEscapingReceivers.where(instructionDominatesBlock), + result.unaliasedAllocations.addAll( + unaliasedAllocations.where(instructionDominatesBlock), ); return result; }
diff --git a/tests/web/regress/issue/63243a_test.dart b/tests/web/regress/issue/63243a_test.dart new file mode 100644 index 0000000..e5ed818 --- /dev/null +++ b/tests/web/regress/issue/63243a_test.dart
@@ -0,0 +1,40 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:expect/expect.dart'; + +class Thing { + int value = 0; +} + +bool get alwaysTrue => DateTime.now().millisecondsSinceEpoch > 42; + +void test1() { + final t1 = Thing(); + final t2 = Thing(); + final t3 = alwaysTrue ? t1 : t2; + t3.value++; + + Expect.equals('[1, 0, 1]', [t1.value, t2.value, t3.value].toString()); +} + +void test2() { + final t1 = Thing(); + final t2 = Thing(); + final t3 = alwaysTrue ? t1 : t2; + escape(t3); + + Expect.equals('[1, 0, 1]', [t1.value, t2.value, t3.value].toString()); +} + +@pragma('dart2js:never-inline') +void escape(Thing thing) { + thing.value++; +} + +void main() { + for (final test in [test1, test2]) { + test(); + } +}