Skip to content

Commit ce8a8b2

Browse files
alexmarkovCommit Queue
authored andcommitted
[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 <[email protected]> Reviewed-by: Slava Egorov <[email protected]>
1 parent 4a52cbc commit ce8a8b2

File tree

2 files changed

+41
-20
lines changed

2 files changed

+41
-20
lines changed

runtime/tests/vm/dart/align_loops_verify_alignment_test.dart

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
// OtherResources=align_loops_test.dart
6+
57
import 'dart:convert';
68
import 'dart:io';
79
import 'dart:async';
@@ -21,12 +23,20 @@ void checkAligned(Symbol sym) {
2123
}
2224
}
2325

24-
Future<void> testAOT(String dillPath, {bool useAsm = false}) async {
25-
await withTempDir('align-loops-test-${useAsm ? 'asm' : 'elf'}',
26-
(String tempDir) async {
26+
Future<void> testAOT(
27+
String dillPath, {
28+
required bool useAsm,
29+
required bool testILSerialization,
30+
}) async {
31+
await withTempDir('align-loops-test-${useAsm ? 'asm' : 'elf'}', (
32+
String tempDir,
33+
) async {
2734
// Generate the snapshot
2835
final snapshotPath = path.join(tempDir, 'libtest.so');
29-
final commonSnapshotArgs = [dillPath];
36+
final commonSnapshotArgs = [
37+
if (testILSerialization) '--test_il_serialization',
38+
dillPath,
39+
];
3040

3141
if (useAsm) {
3242
final assemblyPath = path.join(tempDir, 'test.S');
@@ -68,24 +78,33 @@ void main() async {
6878

6979
await withTempDir('align_loops', (String tempDir) async {
7080
final testProgram = path.join(
71-
sdkDir, 'runtime', 'tests', 'vm', 'dart', 'align_loops_test.dart');
81+
sdkDir,
82+
'runtime',
83+
'tests',
84+
'vm',
85+
'dart',
86+
'align_loops_test.dart',
87+
);
7288

7389
final aotDillPath = path.join(tempDir, 'aot_test.dill');
7490
await run(genKernel, <String>[
7591
'--aot',
7692
'--platform',
7793
platformDill,
78-
...Platform.executableArguments
79-
.where((arg) => arg.startsWith('--enable-experiment=')),
94+
...Platform.executableArguments.where(
95+
(arg) => arg.startsWith('--enable-experiment='),
96+
),
8097
'-o',
8198
aotDillPath,
82-
testProgram
99+
testProgram,
83100
]);
84101

85102
await Future.wait([
86103
// Test unstripped ELF generation directly.
87-
testAOT(aotDillPath),
88-
testAOT(aotDillPath, useAsm: true),
104+
testAOT(aotDillPath, useAsm: false, testILSerialization: false),
105+
testAOT(aotDillPath, useAsm: true, testILSerialization: false),
106+
testAOT(aotDillPath, useAsm: false, testILSerialization: true),
107+
testAOT(aotDillPath, useAsm: true, testILSerialization: true),
89108
]);
90109
});
91110
}

runtime/vm/compiler/backend/flow_graph_compiler.cc

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -663,7 +663,17 @@ static bool IsMarkedWithAlignLoops(const Function& function) {
663663

664664
void FlowGraphCompiler::VisitBlocks() {
665665
CompactBlocks();
666-
if (compiler::Assembler::EmittingComments()) {
666+
667+
#if defined(TARGET_ARCH_X64) || defined(TARGET_ARCH_ARM64)
668+
const bool should_align_loops =
669+
(FLAG_align_all_loops || IsMarkedWithAlignLoops(function())) &&
670+
(kPreferredLoopAlignment > 1);
671+
#else
672+
static_assert(kPreferredLoopAlignment == 1);
673+
const bool should_align_loops = false;
674+
#endif // defined(TARGET_ARCH_X64) || defined(TARGET_ARCH_ARM64)
675+
676+
if (should_align_loops || compiler::Assembler::EmittingComments()) {
667677
// The loop_info fields were cleared, recompute.
668678
flow_graph().ComputeLoops();
669679
}
@@ -679,11 +689,6 @@ void FlowGraphCompiler::VisitBlocks() {
679689
const auto inner_lr_state = ComputeInnerLRState(flow_graph());
680690
#endif // defined(TARGET_ARCH_ARM) || defined(TARGET_ARCH_ARM64)
681691

682-
#if defined(TARGET_ARCH_X64) || defined(TARGET_ARCH_ARM64)
683-
const bool should_align_loops =
684-
FLAG_align_all_loops || IsMarkedWithAlignLoops(function());
685-
#endif // defined(TARGET_ARCH_X64) || defined(TARGET_ARCH_ARM64)
686-
687692
for (intptr_t i = 0; i < block_order().length(); ++i) {
688693
// Compile the block entry.
689694
BlockEntryInstr* entry = block_order()[i];
@@ -720,13 +725,10 @@ void FlowGraphCompiler::VisitBlocks() {
720725
}
721726

722727
#if defined(TARGET_ARCH_X64) || defined(TARGET_ARCH_ARM64)
723-
if (should_align_loops && entry->IsLoopHeader() &&
724-
kPreferredLoopAlignment > 1) {
728+
if (should_align_loops && entry->IsLoopHeader()) {
725729
assembler()->mark_should_be_aligned();
726730
assembler()->Align(kPreferredLoopAlignment, 0);
727731
}
728-
#else
729-
static_assert(kPreferredLoopAlignment == 1);
730732
#endif // defined(TARGET_ARCH_X64) || defined(TARGET_ARCH_ARM64)
731733

732734
BeginCodeSourceRange(entry->source());

0 commit comments

Comments
 (0)