Skip to content

Commit 3b9b779

Browse files
sstricklcommit-bot@chromium.org
authored andcommitted
Reland "[vm/compiler] Move AssertAssignables out of closure bodies."
This is a reland of 42c76fd Main issue was due to the parameter order of the invoke field dispatcher not matching the argument order described by its saved arguments descriptor. There's no reason for it not to, so now it does. Also fixes some issues with stack trace tests that failed due to increased deduplication of closures by forbidding deduplication for those tests. TEST=Run on trybots of all architectures as well as flutter engine trybot, new test added for downstream issues seen after initial landing. Original change's description: > [vm/compiler] Move AssertAssignables out of closure bodies. > > This CL moves the final set of checks out of closure bodies and into > dynamic closure call dispatchers. It also adds stubs for checking top > types and null assignability for types only known at runtime. > > Fixes #40813 . > > Changes in Flutter gallery in release mode: > > * arm7: -3.05% total, +0.99% vmisolate, -0.89% isolate, > -1.20% readonly, -4.43% instructions > * arm8: -3.20% total, +0.99% vmisolate, -0.88% isolate, > -1.18% readonly, -5.05% instructions > > TEST=Run on trybots of all architectures, includes test adjustments where needed. > > Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-nnbd-linux-debug-ia32-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-nnbd-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-nnbd-linux-release-simarm-try,vm-kernel-nnbd-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-nnbd-linux-release-simarm64-try > Change-Id: Ifb136c64339be76a642ecbb4fda26b6ce8f871f9 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/166622 > Commit-Queue: Tess Strickland <[email protected]> > Reviewed-by: Martin Kustermann <[email protected]> > Reviewed-by: Régis Crelier <[email protected]> Change-Id: Ic5ec59cf355f7779bb82db798d97d762ba1e5556 Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-nnbd-linux-debug-ia32-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-nnbd-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-nnbd-linux-release-simarm-try,vm-kernel-nnbd-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-nnbd-linux-release-simarm64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-linux-product-x64-try,flutter-engine-linux-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/172644 Commit-Queue: Tess Strickland <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
1 parent 1d11376 commit 3b9b779

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+929
-627
lines changed

pkg/expect/lib/expect.dart

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,17 @@ class Expect {
440440
_fail("$defaultMessage$diff");
441441
}
442442

443+
/// Checks that [haystack] contains a given substring [needle].
444+
///
445+
/// For example, this succeeds:
446+
///
447+
/// Expect.contains("a", "abcdefg");
448+
static void contains(String needle, String haystack) {
449+
if (!haystack.contains(needle)) {
450+
_fail("String '$needle' not found within '$haystack'");
451+
}
452+
}
453+
443454
/// Checks that [actual] contains a given list of [substrings] in order.
444455
///
445456
/// For example, this succeeds:

runtime/tests/vm/dart/causal_stacks/async_throws_stack_lazy_non_symbolic_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
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-
// VMOptions=--dwarf-stack-traces --save-debugging-info=async_lazy_debug.so --lazy-async-stacks --no-causal-async-stacks
5+
// VMOptions=--dwarf-stack-traces --save-debugging-info=async_lazy_debug.so --lazy-async-stacks --no-causal-async-stacks --no-use-bare-instructions
66

77
import 'dart:async';
88
import 'dart:io';

runtime/tests/vm/dart/causal_stacks/utils.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1355,7 +1355,7 @@ Future<void> doTestsLazy([String? debugInfoFilename]) async {
13551355
final awaitTimeoutExpected = const <String>[
13561356
r'^#0 throwAsync \(.*/utils.dart:21(:3)?\)$',
13571357
r'^<asynchronous suspension>$',
1358-
r'^#1 Future.timeout.<anonymous closure> \(dart:async/future_impl.dart\)$',
1358+
r'^#1 Future.timeout.<anonymous closure> \(dart:async/future_impl.dart',
13591359
r'^<asynchronous suspension>$',
13601360
r'^#2 awaitTimeout ',
13611361
r'^<asynchronous suspension>$',
@@ -1386,7 +1386,7 @@ Future<void> doTestsLazy([String? debugInfoFilename]) async {
13861386
final awaitWaitExpected = const <String>[
13871387
r'^#0 throwAsync \(.*/utils.dart:21(:3)?\)$',
13881388
r'^<asynchronous suspension>$',
1389-
r'^#1 Future.wait.<anonymous closure> \(dart:async/future.dart\)$',
1389+
r'^#1 Future.wait.<anonymous closure> \(dart:async/future.dart',
13901390
r'^<asynchronous suspension>$',
13911391
r'^#2 awaitWait ',
13921392
r'^<asynchronous suspension>$',

runtime/tests/vm/dart/entrypoints/tearoff_prologue_test.dart

Lines changed: 0 additions & 39 deletions
This file was deleted.

runtime/tests/vm/dart/entrypoints/tearoff_test.dart

Lines changed: 0 additions & 38 deletions
This file was deleted.

runtime/tests/vm/dart_2/causal_stacks/async_throws_stack_lazy_non_symbolic_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
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-
// VMOptions=--dwarf-stack-traces --save-debugging-info=async_lazy_debug.so --lazy-async-stacks --no-causal-async-stacks
5+
// VMOptions=--dwarf-stack-traces --save-debugging-info=async_lazy_debug.so --lazy-async-stacks --no-causal-async-stacks --no-use-bare-instructions
66

77
import 'dart:async';
88
import 'dart:io';

runtime/tests/vm/dart_2/causal_stacks/utils.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1355,7 +1355,7 @@ Future<void> doTestsLazy([String debugInfoFilename]) async {
13551355
final awaitTimeoutExpected = const <String>[
13561356
r'^#0 throwAsync \(.*/utils.dart:21(:3)?\)$',
13571357
r'^<asynchronous suspension>$',
1358-
r'^#1 Future.timeout.<anonymous closure> \(dart:async/future_impl.dart\)$',
1358+
r'^#1 Future.timeout.<anonymous closure> \(dart:async/future_impl.dart',
13591359
r'^<asynchronous suspension>$',
13601360
r'^#2 awaitTimeout ',
13611361
r'^<asynchronous suspension>$',
@@ -1386,7 +1386,7 @@ Future<void> doTestsLazy([String debugInfoFilename]) async {
13861386
final awaitWaitExpected = const <String>[
13871387
r'^#0 throwAsync \(.*/utils.dart:21(:3)?\)$',
13881388
r'^<asynchronous suspension>$',
1389-
r'^#1 Future.wait.<anonymous closure> \(dart:async/future.dart\)$',
1389+
r'^#1 Future.wait.<anonymous closure> \(dart:async/future.dart',
13901390
r'^<asynchronous suspension>$',
13911391
r'^#2 awaitWait ',
13921392
r'^<asynchronous suspension>$',

runtime/tests/vm/dart_2/entrypoints/tearoff_prologue_test.dart

Lines changed: 0 additions & 39 deletions
This file was deleted.

runtime/tests/vm/dart_2/entrypoints/tearoff_test.dart

Lines changed: 0 additions & 38 deletions
This file was deleted.

runtime/vm/compiler/assembler/assembler_arm.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,6 +1023,12 @@ class Assembler : public AssemblerBase {
10231023
JumpDistance distance = kFarJump) {
10241024
b(label, condition);
10251025
}
1026+
void BranchIfZero(Register rn,
1027+
Label* label,
1028+
JumpDistance distance = kFarJump) {
1029+
cmp(rn, Operand(0));
1030+
b(label, ZERO);
1031+
}
10261032

10271033
void MoveRegister(Register rd, Register rm, Condition cond = AL);
10281034

runtime/vm/compiler/assembler/assembler_arm64.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,6 +1109,11 @@ class Assembler : public AssemblerBase {
11091109
JumpDistance distance = kFarJump) {
11101110
b(label, condition);
11111111
}
1112+
void BranchIfZero(Register rn,
1113+
Label* label,
1114+
JumpDistance distance = kFarJump) {
1115+
cbz(label, rn);
1116+
}
11121117

11131118
void cbz(Label* label, Register rt, OperandSize sz = kEightBytes) {
11141119
EmitCompareAndBranch(CBZ, rt, label, sz);

runtime/vm/compiler/assembler/assembler_ia32.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1784,10 +1784,12 @@ void Assembler::LoadFromOffset(Register reg,
17841784
}
17851785

17861786
void Assembler::LoadFromStack(Register dst, intptr_t depth) {
1787+
ASSERT(depth >= 0);
17871788
movl(dst, Address(ESP, depth * target::kWordSize));
17881789
}
17891790

17901791
void Assembler::StoreToStack(Register src, intptr_t depth) {
1792+
ASSERT(depth >= 0);
17911793
movl(Address(ESP, depth * target::kWordSize), src);
17921794
}
17931795

runtime/vm/compiler/assembler/assembler_ia32.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,12 @@ class Assembler : public AssemblerBase {
577577
JumpDistance distance = kFarJump) {
578578
j(condition, label, distance);
579579
}
580+
void BranchIfZero(Register src,
581+
Label* label,
582+
JumpDistance distance = kFarJump) {
583+
cmpl(src, Immediate(0));
584+
j(ZERO, label, distance);
585+
}
580586

581587
void LoadFromOffset(Register reg,
582588
Register base,
@@ -716,6 +722,11 @@ class Assembler : public AssemblerBase {
716722
cmpxchgl(address, reg);
717723
}
718724

725+
void CompareTypeNullabilityWith(Register type, int8_t value) {
726+
cmpb(FieldAddress(type, compiler::target::Type::nullability_offset()),
727+
Immediate(value));
728+
}
729+
719730
void EnterFrame(intptr_t frame_space);
720731
void LeaveFrame();
721732
void ReserveAlignedFrameSpace(intptr_t frame_space);

runtime/vm/compiler/assembler/assembler_x64.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,12 @@ class Assembler : public AssemblerBase {
683683
JumpDistance distance = kFarJump) {
684684
j(condition, label, distance);
685685
}
686+
void BranchIfZero(Register src,
687+
Label* label,
688+
JumpDistance distance = kFarJump) {
689+
cmpq(src, Immediate(0));
690+
j(ZERO, label, distance);
691+
}
686692

687693
// Issues a move instruction if 'to' is not the same as 'from'.
688694
void MoveRegister(Register to, Register from);

0 commit comments

Comments
 (0)