Skip to content

Commit 0dc8685

Browse files
sstricklCommit Queue
authored and
Commit Queue
committed
Reland "[vm/compiler] Consistently use PointerBase.data values as FFiIntPtr."
This is a reland of commit 60d1a40 Original change's description: > Reland "[vm/compiler] Consistently use PointerBase.data values as FFiIntPtr." > > This is a reland of commit 5c4fd50 > > The range analysis change was replaced with a different change, > on which this CL is based. In addition, another preceding CL > unified a few methods on subclasses of UnboxIntegerOpInstr, because > the inconsistencies between UnboxInt32OpInstr and UnboxUint32OpInstr > and how unboxed int32 vs. uint32 constants were handled caused the > issue seen after landing the original CL. > > Thus, this CL is purely changes of kUnboxedIntPtr to kUnboxedFfiIntPtr > when appropriate plus test changes. > > TEST=vm/dart/regress_306327173_il_test > vm/dart/address_local_pointer_il_test > > Original change's description: > > [vm/compiler] Consistently use PointerBase.data values as FFiIntPtr. > > > > Also if converting an unboxed int with only non-negative values > > that fit in 32 bits to a uint32, then keep the range from the value. > > > > TEST=regress_306327173_il_test > > > > Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-simarm_x64-try,vm-aot-linux-release-x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try > > Change-Id: Id9e7c2d5f477e560822a02574739c57d77b5a6d1 > > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/332202 > > Reviewed-by: Daco Harkes <[email protected]> > > Reviewed-by: Slava Egorov <[email protected]> > > Commit-Queue: Tess Strickland <[email protected]> > > Change-Id: I1f66bc9d1ca7569c913f02b611b4a27f189605ac > Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-simarm_x64-try,vm-aot-linux-release-x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-android-release-arm-try > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/332340 > Reviewed-by: Daco Harkes <[email protected]> > Commit-Queue: Tess Strickland <[email protected]> TEST=vm/dart/regress_306327173_il_test vm/dart/address_local_pointer_il_test Change-Id: I8f789847670c66a823cbbc6dca5b74a8b7a2cd88 Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-simarm_x64-try,vm-aot-linux-release-x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-android-release-arm-try,vm-linux-release-simarm-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/333002 Reviewed-by: Daco Harkes <[email protected]> Commit-Queue: Tess Strickland <[email protected]>
1 parent 3a6daed commit 0dc8685

File tree

3 files changed

+32
-47
lines changed

3 files changed

+32
-47
lines changed

runtime/tests/vm/dart/address_local_pointer_il_test.dart

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,32 +17,21 @@ import 'package:vm/testing/il_matchers.dart';
1717
int identity(int address) => Pointer<Void>.fromAddress(address).address;
1818

1919
void matchIL$identity(FlowGraph graph) {
20-
graph.dump();
21-
if (is32BitConfiguration) {
22-
// The Dart int address is truncated before being returned.
23-
graph.match([
24-
match.block('Graph'),
25-
match.block('Function', [
26-
'address' << match.Parameter(index: 0),
27-
'int32' <<
28-
match.IntConverter('address',
29-
from: 'int64', to: 'int32', is_truncating: true),
20+
final retval = is32BitConfiguration ? 'retval' : 'address';
21+
graph.match([
22+
match.block('Graph'),
23+
match.block('Function', [
24+
'address' << match.Parameter(index: 0),
25+
if (is32BitConfiguration) ...[
26+
// The Dart int address is truncated before being returned.
3027
'uint32' <<
31-
match.IntConverter('int32',
32-
from: 'int32', to: 'uint32', is_truncating: true),
28+
match.IntConverter('address',
29+
from: 'int64', to: 'uint32', is_truncating: true),
3330
'retval' << match.IntConverter('uint32', from: 'uint32', to: 'int64'),
34-
match.Return('retval'),
35-
]),
36-
]);
37-
} else {
38-
graph.match([
39-
match.block('Graph'),
40-
match.block('Function', [
41-
'address' << match.Parameter(index: 0),
42-
match.Return('address'),
43-
]),
44-
]);
45-
}
31+
],
32+
match.Return(retval),
33+
]),
34+
]);
4635
}
4736

4837
void main(List<String> args) {

runtime/tests/vm/dart/regress_306327173_il_test.dart

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,7 @@ void matchIL$deref(FlowGraph graph) {
3030
// and int64 on 64-bit arches.
3131
if (is32BitConfiguration) ...[
3232
// 'unboxed' needs to be converted to int64 before returning.
33-
//
34-
// Note: The first two conversions here should be fixed once all
35-
// kUnboxedIntPtr uses are appropriately converted to kUnboxedFfiIntPtr.
36-
'extra1' << match.IntConverter('unboxed', from: 'uint32', to: 'int32'),
37-
'extra2' << match.IntConverter('extra1', from: 'int32', to: 'uint32'),
38-
'address' << match.IntConverter('extra2', from: 'uint32', to: 'int64'),
33+
'address' << match.IntConverter('unboxed', from: 'uint32', to: 'int64'),
3934
],
4035
match.Return(retvalName),
4136
]),

runtime/vm/compiler/frontend/kernel_to_il.cc

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,8 +1384,8 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod(
13841384
LocalVariable* pointer = MakeTemporary();
13851385
body += LoadLocal(pointer);
13861386
body += LoadLocal(address);
1387-
body += UnboxTruncate(kUnboxedIntPtr);
1388-
body += ConvertUnboxedToUntagged(kUnboxedIntPtr);
1387+
body += UnboxTruncate(kUnboxedFfiIntPtr);
1388+
body += ConvertUnboxedToUntagged(kUnboxedFfiIntPtr);
13891389
body += StoreNativeField(Slot::PointerBase_data(),
13901390
InnerPointerAccess::kCannotBeInnerPointer,
13911391
StoreFieldInstr::Kind::kInitializing);
@@ -1467,8 +1467,8 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod(
14671467
body += LoadLocal(MakeTemporary()); // Duplicate Pointer.
14681468
body += LoadLocal(parsed_function_->RawParameterVariable(0)); // Address.
14691469
body += CheckNullOptimized(String::ZoneHandle(Z, function.name()));
1470-
body += UnboxTruncate(kUnboxedIntPtr);
1471-
body += ConvertUnboxedToUntagged(kUnboxedIntPtr);
1470+
body += UnboxTruncate(kUnboxedFfiIntPtr);
1471+
body += ConvertUnboxedToUntagged(kUnboxedFfiIntPtr);
14721472
body += StoreNativeField(Slot::PointerBase_data(),
14731473
InnerPointerAccess::kCannotBeInnerPointer,
14741474
StoreFieldInstr::Kind::kInitializing);
@@ -1800,11 +1800,12 @@ Fragment FlowGraphBuilder::BuildTypedDataViewFactoryConstructor(
18001800
body += LoadLocal(typed_data);
18011801
body += LoadNativeField(Slot::PointerBase_data(),
18021802
InnerPointerAccess::kMayBeInnerPointer);
1803-
body += ConvertUntaggedToUnboxed(kUnboxedIntPtr);
1803+
body += ConvertUntaggedToUnboxed(kUnboxedFfiIntPtr);
18041804
body += LoadLocal(offset_in_bytes);
1805-
body += UnboxTruncate(kUnboxedIntPtr);
1806-
body += BinaryIntegerOp(Token::kADD, kUnboxedIntPtr, /*is_truncating=*/true);
1807-
body += ConvertUnboxedToUntagged(kUnboxedIntPtr);
1805+
body += UnboxTruncate(kUnboxedFfiIntPtr);
1806+
body +=
1807+
BinaryIntegerOp(Token::kADD, kUnboxedFfiIntPtr, /*is_truncating=*/true);
1808+
body += ConvertUnboxedToUntagged(kUnboxedFfiIntPtr);
18081809
body += StoreNativeField(Slot::PointerBase_data(),
18091810
InnerPointerAccess::kMayBeInnerPointer,
18101811
StoreFieldInstr::Kind::kInitializing);
@@ -1865,27 +1866,27 @@ Fragment FlowGraphBuilder::BuildTypedDataMemMove(const Function& function,
18651866
call_memmove += LoadLocal(arg_to);
18661867
call_memmove += LoadNativeField(Slot::PointerBase_data(),
18671868
InnerPointerAccess::kMayBeInnerPointer);
1868-
call_memmove += ConvertUntaggedToUnboxed(kUnboxedIntPtr);
1869+
call_memmove += ConvertUntaggedToUnboxed(kUnboxedFfiIntPtr);
18691870
call_memmove += LoadLocal(arg_to_start);
18701871
call_memmove += IntConstant(element_size);
18711872
call_memmove += SmiBinaryOp(Token::kMUL, /*is_truncating=*/true);
1872-
call_memmove += UnboxTruncate(kUnboxedIntPtr);
1873+
call_memmove += UnboxTruncate(kUnboxedFfiIntPtr);
18731874
call_memmove +=
1874-
BinaryIntegerOp(Token::kADD, kUnboxedIntPtr, /*is_truncating=*/true);
1875+
BinaryIntegerOp(Token::kADD, kUnboxedFfiIntPtr, /*is_truncating=*/true);
18751876
call_memmove += LoadLocal(arg_from);
18761877
call_memmove += LoadNativeField(Slot::PointerBase_data(),
18771878
InnerPointerAccess::kMayBeInnerPointer);
1878-
call_memmove += ConvertUntaggedToUnboxed(kUnboxedIntPtr);
1879+
call_memmove += ConvertUntaggedToUnboxed(kUnboxedFfiIntPtr);
18791880
call_memmove += LoadLocal(arg_from_start);
18801881
call_memmove += IntConstant(element_size);
18811882
call_memmove += SmiBinaryOp(Token::kMUL, /*is_truncating=*/true);
1882-
call_memmove += UnboxTruncate(kUnboxedIntPtr);
1883+
call_memmove += UnboxTruncate(kUnboxedFfiIntPtr);
18831884
call_memmove +=
1884-
BinaryIntegerOp(Token::kADD, kUnboxedIntPtr, /*is_truncating=*/true);
1885+
BinaryIntegerOp(Token::kADD, kUnboxedFfiIntPtr, /*is_truncating=*/true);
18851886
call_memmove += LoadLocal(arg_count);
18861887
call_memmove += IntConstant(element_size);
18871888
call_memmove += SmiBinaryOp(Token::kMUL, /*is_truncating=*/true);
1888-
call_memmove += UnboxTruncate(kUnboxedIntPtr);
1889+
call_memmove += UnboxTruncate(kUnboxedFfiIntPtr);
18891890
call_memmove += LoadThread();
18901891
call_memmove += LoadUntagged(
18911892
compiler::target::Thread::OffsetFromThread(&kMemoryMoveRuntimeEntry));
@@ -4529,8 +4530,8 @@ Fragment FlowGraphBuilder::FfiPointerFromAddress() {
45294530
LocalVariable* pointer = MakeTemporary();
45304531
code += LoadLocal(pointer);
45314532
code += LoadLocal(address);
4532-
code += UnboxTruncate(kUnboxedIntPtr);
4533-
code += ConvertUnboxedToUntagged(kUnboxedIntPtr);
4533+
code += UnboxTruncate(kUnboxedFfiIntPtr);
4534+
code += ConvertUnboxedToUntagged(kUnboxedFfiIntPtr);
45344535
code += StoreNativeField(Slot::PointerBase_data(),
45354536
InnerPointerAccess::kCannotBeInnerPointer,
45364537
StoreFieldInstr::Kind::kInitializing);

0 commit comments

Comments
 (0)