From da1f06f239bef692cc25b5d985df59fe0afe57ec Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Fri, 8 May 2026 06:37:30 -0700 Subject: [PATCH] [vm,compiler] Fix loop computation in case of try blocks If catch block is included into the loop, we should also include the whole try body as there are implicit control flow edges between each block in a try body and corresponding catch block. If try body is not included, analysis of loop invariant loads could miss conflicting stores and may conclude that certain loads are loop invariant although they are not. TEST=runtime/tests/vm/dart/regress_63336_test.dart Fixes https://github.com/dart-lang/sdk/issues/63336 Change-Id: I9d6b73e24554ee3bfbcc722fa59634ea90b00abb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/501560 Reviewed-by: Slava Egorov Commit-Queue: Alexander Markov --- pkg/cfg/lib/ir/loops.dart | 9 ++++ pkg/cfg/testcases/loops.dart | 22 +++++++++ pkg/cfg/testcases/loops.dart.expect | 45 +++++++++++++++++++ runtime/tests/vm/dart/regress_63336_test.dart | 43 ++++++++++++++++++ runtime/vm/compiler/backend/flow_graph.cc | 12 +++++ 5 files changed, 131 insertions(+) create mode 100644 runtime/tests/vm/dart/regress_63336_test.dart diff --git a/pkg/cfg/lib/ir/loops.dart b/pkg/cfg/lib/ir/loops.dart index 84b8bbcfaae..a5dd52be365 100644 --- a/pkg/cfg/lib/ir/loops.dart +++ b/pkg/cfg/lib/ir/loops.dart @@ -61,6 +61,15 @@ class Loop { workList.add(pred); } } + if (block is CatchBlock) { + // Blocks from the try body are implicit predecessors of a catch block. + for (final pred in block.graph.reversePostorder) { + if (pred.exceptionHandler == block && !contains(pred)) { + add(pred); + workList.add(pred); + } + } + } } } diff --git a/pkg/cfg/testcases/loops.dart b/pkg/cfg/testcases/loops.dart index 2bcaf19537b..a318480590e 100644 --- a/pkg/cfg/testcases/loops.dart +++ b/pkg/cfg/testcases/loops.dart @@ -41,4 +41,26 @@ void irreducible(int i) { } } +int var63 = 28; +int var68 = 44; + +void withTryBlock() { + var n = 43; + while (--n > 0) { + try { + var63++; + // Terminate block without reaching a loop backedge, + // so try body won't be included into the loop body + // through explicit predecessors of the backedge. + throw 'bye'; + } on StackOverflowError { + rethrow; + } catch (_) { + // Load from 'var63' is considered loop invariant if + // loop body doesn't include try block body. + var68 = var63; + } + } +} + void main() {} diff --git a/pkg/cfg/testcases/loops.dart.expect b/pkg/cfg/testcases/loops.dart.expect index 568873ffb41..d6708ffc3e9 100644 --- a/pkg/cfg/testcases/loops.dart.expect +++ b/pkg/cfg/testcases/loops.dart.expect @@ -111,8 +111,53 @@ B14 = TargetBlock() idom:B8 B34 = JoinBlock(B14, B29) idom:B0 Return(v38) +--- withTryBlock +B0 = EntryBlock() dominates:(B3) + v1 = Constant(43) + v6 = Constant(1) + v9 = Constant(0) + v20 = Constant("bye") + v22 = Constant(null) + v42 = Constant(-1) + Goto(B3) +B3 = JoinBlock(B0, B28) idom:B0 dominates:(B12, B11) loop-header (depth:1 body:(B3, B11, B15, B28, B14) back-edges:(B28)) + v41 = Phi(v1, v43) + v43 = BinaryIntOp +(v41, v42) + v10 = Comparison int >(v43, v9) + Branch(v10, true: B11, false: B12) +B11 = TargetBlock() idom:B3 dominates:(B15, B14) in-loop:B3 + TryEntry(try-body: B14, catch-block: B15) +B14 = TargetBlock() exception-handler:B15 idom:B11 in-loop:B3 + v17 = LoadStaticField(var63) + v18 = BinaryIntOp +(v17, v6) + StoreStaticField(var63, v18) + Throw(v20) +B15 = CatchBlock() idom:B11 dominates:(B28, B27) in-loop:B3 + v23 = Parameter(#exception) + v24 = Parameter(#stackTrace) + v26 = TypeTest(v23, StackOverflowError) + Branch(v26, true: B27, false: B28) +B27 = TargetBlock() idom:B15 + Throw(v23, v24) +B28 = TargetBlock() idom:B15 in-loop:B3 + v35 = LoadStaticField(var63) + StoreStaticField(var68, v35) + Goto(B3) +B12 = TargetBlock() idom:B3 + Return(v22) + --- main B0 = EntryBlock() v1 = Constant(null) Return(v1) +--- field-init var63 +B0 = EntryBlock() + v1 = Constant(28) + Return(v1) + +--- field-init var68 +B0 = EntryBlock() + v1 = Constant(44) + Return(v1) + diff --git a/runtime/tests/vm/dart/regress_63336_test.dart b/runtime/tests/vm/dart/regress_63336_test.dart new file mode 100644 index 00000000000..c6e08f07c21 --- /dev/null +++ b/runtime/tests/vm/dart/regress_63336_test.dart @@ -0,0 +1,43 @@ +// 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. + +// Verify that a load from a catch block is not considered loop invariant +// and not hoisted out of the loop if there is a store in the try body. +// Regression test for https://github.com/dart-lang/sdk/issues/63336. + +// VMOptions=--optimization-counter-threshold=100 --no-background-compilation + +import 'package:expect/expect.dart'; + +int var63 = 28; +int var68 = 44; + +void test() { + int n = 43; + while (--n > 0) { + try { + var63++; + // Terminate block without reaching a loop backedge, + // so try body won't be included into the loop body + // through explicit predecessors of the backedge. + throw 'bye'; + // Make sure load is not immediately in a CatchBlockEntry. + } on StackOverflowError { + rethrow; + } catch (_) { + // Load from 'var63' is considered loop invariant if + // loop body doesn't include try block body. + var68 = var63; + } + } +} + +void main() { + for (int i = 0; i < 200; ++i) { + var63 = 28; + var68 = 44; + test(); + Expect.equals(70, var68); + } +} diff --git a/runtime/vm/compiler/backend/flow_graph.cc b/runtime/vm/compiler/backend/flow_graph.cc index c8a0db47ff2..732b6df5c5b 100644 --- a/runtime/vm/compiler/backend/flow_graph.cc +++ b/runtime/vm/compiler/backend/flow_graph.cc @@ -2089,6 +2089,18 @@ BitVector* FlowGraph::FindLoopBlocks(BlockEntryInstr* m, stack.Add(q); } } + if (auto catch_entry = p->AsCatchBlockEntry()) { + // Blocks from the try body are implicit predecessors of a catch block. + const intptr_t try_index = catch_entry->catch_try_index(); + for (auto block : reverse_postorder()) { + if (block->try_index() == try_index) { + if (!loop_blocks->Contains(block->preorder_number())) { + loop_blocks->Add(block->preorder_number()); + stack.Add(block); + } + } + } + } } return loop_blocks; }