From ce8a8b29b62c5ad3b6bee27d8aee10ff7ca89023 Mon Sep 17 00:00:00 2001
From: Alexander Markov <alexmarkov@google.com>
Date: Fri, 20 Dec 2024 05:49:02 -0800
Subject: [PATCH] [vm,compiler] Recalculate loops when aligning loop headers

Loop information may be outdated by the time code is generated
(for example, if flow graph is serialized/deserialized).

Recalculate loop information during code generation if it is needed
in order to align loop headers.

TEST=vm/dart/align_loops_verify_alignment_test

Change-Id: I1edd63354cee9940bbd474dc95535b4b74081830
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401863
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
---
 .../align_loops_verify_alignment_test.dart    | 39 ++++++++++++++-----
 .../compiler/backend/flow_graph_compiler.cc   | 22 ++++++-----
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/runtime/tests/vm/dart/align_loops_verify_alignment_test.dart b/runtime/tests/vm/dart/align_loops_verify_alignment_test.dart
index be06e9a6cc0..d41a13c491b 100644
--- a/runtime/tests/vm/dart/align_loops_verify_alignment_test.dart
+++ b/runtime/tests/vm/dart/align_loops_verify_alignment_test.dart
@@ -2,6 +2,8 @@
 // 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.
 
+// OtherResources=align_loops_test.dart
+
 import 'dart:convert';
 import 'dart:io';
 import 'dart:async';
@@ -21,12 +23,20 @@ void checkAligned(Symbol sym) {
   }
 }
 
-Future<void> testAOT(String dillPath, {bool useAsm = false}) async {
-  await withTempDir('align-loops-test-${useAsm ? 'asm' : 'elf'}',
-      (String tempDir) async {
+Future<void> testAOT(
+  String dillPath, {
+  required bool useAsm,
+  required bool testILSerialization,
+}) async {
+  await withTempDir('align-loops-test-${useAsm ? 'asm' : 'elf'}', (
+    String tempDir,
+  ) async {
     // Generate the snapshot
     final snapshotPath = path.join(tempDir, 'libtest.so');
-    final commonSnapshotArgs = [dillPath];
+    final commonSnapshotArgs = [
+      if (testILSerialization) '--test_il_serialization',
+      dillPath,
+    ];
 
     if (useAsm) {
       final assemblyPath = path.join(tempDir, 'test.S');
@@ -68,24 +78,33 @@ void main() async {
 
   await withTempDir('align_loops', (String tempDir) async {
     final testProgram = path.join(
-        sdkDir, 'runtime', 'tests', 'vm', 'dart', 'align_loops_test.dart');
+      sdkDir,
+      'runtime',
+      'tests',
+      'vm',
+      'dart',
+      'align_loops_test.dart',
+    );
 
     final aotDillPath = path.join(tempDir, 'aot_test.dill');
     await run(genKernel, <String>[
       '--aot',
       '--platform',
       platformDill,
-      ...Platform.executableArguments
-          .where((arg) => arg.startsWith('--enable-experiment=')),
+      ...Platform.executableArguments.where(
+        (arg) => arg.startsWith('--enable-experiment='),
+      ),
       '-o',
       aotDillPath,
-      testProgram
+      testProgram,
     ]);
 
     await Future.wait([
       // Test unstripped ELF generation directly.
-      testAOT(aotDillPath),
-      testAOT(aotDillPath, useAsm: true),
+      testAOT(aotDillPath, useAsm: false, testILSerialization: false),
+      testAOT(aotDillPath, useAsm: true, testILSerialization: false),
+      testAOT(aotDillPath, useAsm: false, testILSerialization: true),
+      testAOT(aotDillPath, useAsm: true, testILSerialization: true),
     ]);
   });
 }
diff --git a/runtime/vm/compiler/backend/flow_graph_compiler.cc b/runtime/vm/compiler/backend/flow_graph_compiler.cc
index 2c1a3a5ffbe..8cb2f6ff07f 100644
--- a/runtime/vm/compiler/backend/flow_graph_compiler.cc
+++ b/runtime/vm/compiler/backend/flow_graph_compiler.cc
@@ -663,7 +663,17 @@ static bool IsMarkedWithAlignLoops(const Function& function) {
 
 void FlowGraphCompiler::VisitBlocks() {
   CompactBlocks();
-  if (compiler::Assembler::EmittingComments()) {
+
+#if defined(TARGET_ARCH_X64) || defined(TARGET_ARCH_ARM64)
+  const bool should_align_loops =
+      (FLAG_align_all_loops || IsMarkedWithAlignLoops(function())) &&
+      (kPreferredLoopAlignment > 1);
+#else
+  static_assert(kPreferredLoopAlignment == 1);
+  const bool should_align_loops = false;
+#endif  // defined(TARGET_ARCH_X64) || defined(TARGET_ARCH_ARM64)
+
+  if (should_align_loops || compiler::Assembler::EmittingComments()) {
     // The loop_info fields were cleared, recompute.
     flow_graph().ComputeLoops();
   }
@@ -679,11 +689,6 @@ void FlowGraphCompiler::VisitBlocks() {
   const auto inner_lr_state = ComputeInnerLRState(flow_graph());
 #endif  // defined(TARGET_ARCH_ARM) || defined(TARGET_ARCH_ARM64)
 
-#if defined(TARGET_ARCH_X64) || defined(TARGET_ARCH_ARM64)
-  const bool should_align_loops =
-      FLAG_align_all_loops || IsMarkedWithAlignLoops(function());
-#endif  // defined(TARGET_ARCH_X64) || defined(TARGET_ARCH_ARM64)
-
   for (intptr_t i = 0; i < block_order().length(); ++i) {
     // Compile the block entry.
     BlockEntryInstr* entry = block_order()[i];
@@ -720,13 +725,10 @@ void FlowGraphCompiler::VisitBlocks() {
     }
 
 #if defined(TARGET_ARCH_X64) || defined(TARGET_ARCH_ARM64)
-    if (should_align_loops && entry->IsLoopHeader() &&
-        kPreferredLoopAlignment > 1) {
+    if (should_align_loops && entry->IsLoopHeader()) {
       assembler()->mark_should_be_aligned();
       assembler()->Align(kPreferredLoopAlignment, 0);
     }
-#else
-    static_assert(kPreferredLoopAlignment == 1);
 #endif  // defined(TARGET_ARCH_X64) || defined(TARGET_ARCH_ARM64)
 
     BeginCodeSourceRange(entry->source());