Skip to content

Commit 20b6843

Browse files
sstricklCommit Queue
sstrickl
authored and
Commit Queue
committed
[vm/compiler] Convert more LoadUntagged -> LoadField.
Missed a couple of LoadUntaggeds for PointerBase.data in the graph intrinsifier during 4d1bdaa. Creates slots for ExternalOneByteString.external_data and ExternalTwoByteString.external_data and uses LoadField with those slots instead of LoadUntagged. TEST=ci Issue: #42072 Fixes: #53124 Change-Id: I900281644c4c42ad303cc7a6121e4c8bb7852cfe Cq-Include-Trybots: luci.dart.try:vm-aot-linux-release-simarm_x64-try,vm-aot-linux-release-x64-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330787 Reviewed-by: Alexander Markov <[email protected]> Commit-Queue: Tess Strickland <[email protected]>
1 parent 1b4ae5a commit 20b6843

File tree

9 files changed

+49
-59
lines changed

9 files changed

+49
-59
lines changed

‎runtime/vm/compiler/backend/il_arm.cc

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -407,16 +407,12 @@ void MemoryCopyInstr::EmitComputeStartPointer(FlowGraphCompiler* compiler,
407407
compiler::target::TwoByteString::data_offset() - kHeapObjectTag;
408408
break;
409409
case kExternalOneByteStringCid:
410-
__ ldr(array_reg,
411-
compiler::FieldAddress(array_reg,
412-
compiler::target::ExternalOneByteString::
413-
external_data_offset()));
410+
__ LoadFromSlot(array_reg, array_reg,
411+
Slot::ExternalOneByteString_external_data());
414412
break;
415413
case kExternalTwoByteStringCid:
416-
__ ldr(array_reg,
417-
compiler::FieldAddress(array_reg,
418-
compiler::target::ExternalTwoByteString::
419-
external_data_offset()));
414+
__ LoadFromSlot(array_reg, array_reg,
415+
Slot::ExternalTwoByteString_external_data());
420416
break;
421417
default:
422418
UNREACHABLE();

‎runtime/vm/compiler/backend/il_arm64.cc

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -308,16 +308,12 @@ void MemoryCopyInstr::EmitComputeStartPointer(FlowGraphCompiler* compiler,
308308
compiler::target::TwoByteString::data_offset() - kHeapObjectTag;
309309
break;
310310
case kExternalOneByteStringCid:
311-
__ ldr(array_reg,
312-
compiler::FieldAddress(array_reg,
313-
compiler::target::ExternalOneByteString::
314-
external_data_offset()));
311+
__ LoadFromSlot(array_reg, array_reg,
312+
Slot::ExternalOneByteString_external_data());
315313
break;
316314
case kExternalTwoByteStringCid:
317-
__ ldr(array_reg,
318-
compiler::FieldAddress(array_reg,
319-
compiler::target::ExternalTwoByteString::
320-
external_data_offset()));
315+
__ LoadFromSlot(array_reg, array_reg,
316+
Slot::ExternalTwoByteString_external_data());
321317
break;
322318
default:
323319
UNREACHABLE();

‎runtime/vm/compiler/backend/il_ia32.cc

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -201,16 +201,12 @@ void MemoryCopyInstr::EmitComputeStartPointer(FlowGraphCompiler* compiler,
201201
compiler::target::TwoByteString::data_offset() - kHeapObjectTag;
202202
break;
203203
case kExternalOneByteStringCid:
204-
__ movl(array_reg,
205-
compiler::FieldAddress(array_reg,
206-
compiler::target::ExternalOneByteString::
207-
external_data_offset()));
204+
__ LoadFromSlot(array_reg, array_reg,
205+
Slot::ExternalOneByteString_external_data());
208206
break;
209207
case kExternalTwoByteStringCid:
210-
__ movl(array_reg,
211-
compiler::FieldAddress(array_reg,
212-
compiler::target::ExternalTwoByteString::
213-
external_data_offset()));
208+
__ LoadFromSlot(array_reg, array_reg,
209+
Slot::ExternalTwoByteString_external_data());
214210
break;
215211
default:
216212
UNREACHABLE();

‎runtime/vm/compiler/backend/il_riscv.cc

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -403,16 +403,12 @@ void MemoryCopyInstr::EmitComputeStartPointer(FlowGraphCompiler* compiler,
403403
compiler::target::TwoByteString::data_offset() - kHeapObjectTag;
404404
break;
405405
case kExternalOneByteStringCid:
406-
__ lx(array_reg,
407-
compiler::FieldAddress(array_reg,
408-
compiler::target::ExternalOneByteString::
409-
external_data_offset()));
406+
__ LoadFromSlot(array_reg, array_reg,
407+
Slot::ExternalOneByteString_external_data());
410408
break;
411409
case kExternalTwoByteStringCid:
412-
__ lx(array_reg,
413-
compiler::FieldAddress(array_reg,
414-
compiler::target::ExternalTwoByteString::
415-
external_data_offset()));
410+
__ LoadFromSlot(array_reg, array_reg,
411+
Slot::ExternalTwoByteString_external_data());
416412
break;
417413
default:
418414
UNREACHABLE();

‎runtime/vm/compiler/backend/il_x64.cc

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -296,16 +296,12 @@ void MemoryCopyInstr::EmitComputeStartPointer(FlowGraphCompiler* compiler,
296296
compiler::target::TwoByteString::data_offset() - kHeapObjectTag;
297297
break;
298298
case kExternalOneByteStringCid:
299-
__ movq(array_reg,
300-
compiler::FieldAddress(array_reg,
301-
compiler::target::ExternalOneByteString::
302-
external_data_offset()));
299+
__ LoadFromSlot(array_reg, array_reg,
300+
Slot::ExternalOneByteString_external_data());
303301
break;
304302
case kExternalTwoByteStringCid:
305-
__ movq(array_reg,
306-
compiler::FieldAddress(array_reg,
307-
compiler::target::ExternalTwoByteString::
308-
external_data_offset()));
303+
__ LoadFromSlot(array_reg, array_reg,
304+
Slot::ExternalTwoByteString_external_data());
309305
break;
310306
default:
311307
UNREACHABLE();

‎runtime/vm/compiler/backend/inliner.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3497,14 +3497,14 @@ static Definition* PrepareInlineStringIndexOp(FlowGraph* flow_graph,
34973497

34983498
// For external strings: Load backing store.
34993499
if (cid == kExternalOneByteStringCid) {
3500-
str = new LoadUntaggedInstr(
3501-
new Value(str),
3502-
compiler::target::ExternalOneByteString::external_data_offset());
3500+
str = new LoadFieldInstr(
3501+
new Value(str), Slot::ExternalOneByteString_external_data(),
3502+
InnerPointerAccess::kCannotBeInnerPointer, call->source());
35033503
cursor = flow_graph->AppendTo(cursor, str, nullptr, FlowGraph::kValue);
35043504
} else if (cid == kExternalTwoByteStringCid) {
3505-
str = new LoadUntaggedInstr(
3506-
new Value(str),
3507-
compiler::target::ExternalTwoByteString::external_data_offset());
3505+
str = new LoadFieldInstr(
3506+
new Value(str), Slot::ExternalTwoByteString_external_data(),
3507+
InnerPointerAccess::kCannotBeInnerPointer, call->source());
35083508
cursor = flow_graph->AppendTo(cursor, str, nullptr, FlowGraph::kValue);
35093509
}
35103510

‎runtime/vm/compiler/backend/slot.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,11 @@ NONNULLABLE_BOXED_NATIVE_SLOTS_LIST(FOR_EACH_NATIVE_SLOT)
212212
AOT_ONLY_UNBOXED_NATIVE_ADDRESS_SLOTS_LIST(V) \
213213
V(Function, UntaggedFunction, entry_point, false, FINAL) \
214214
V(FinalizerBase, UntaggedFinalizerBase, isolate, false, VAR) \
215-
V(PointerBase, UntaggedPointerBase, data, true, VAR)
215+
V(PointerBase, UntaggedPointerBase, data, true, VAR) \
216+
V(ExternalOneByteString, UntaggedExternalOneByteString, external_data, \
217+
false, FINAL) \
218+
V(ExternalTwoByteString, UntaggedExternalTwoByteString, external_data, \
219+
false, FINAL)
216220

217221
// For uses that do not need to know whether a given slot may contain an
218222
// inner pointer to a GC-able object or not. (Generally, such users only need

‎runtime/vm/compiler/graph_intrinsifier.cc

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,9 @@ static bool IntrinsifyArrayGetIndexed(FlowGraph* flow_graph,
225225
Slot::GetLengthFieldForArrayCid(array_cid));
226226

227227
if (IsExternalTypedDataClassId(array_cid)) {
228-
array = builder.AddDefinition(new LoadUntaggedInstr(
229-
new Value(array), target::PointerBase::data_offset()));
228+
array = builder.AddDefinition(new LoadFieldInstr(
229+
new Value(array), Slot::PointerBase_data(),
230+
InnerPointerAccess::kCannotBeInnerPointer, builder.Source()));
230231
}
231232

232233
Definition* result = builder.AddDefinition(new LoadIndexedInstr(
@@ -409,8 +410,9 @@ static bool IntrinsifyArraySetIndexed(FlowGraph* flow_graph,
409410
}
410411

411412
if (IsExternalTypedDataClassId(array_cid)) {
412-
array = builder.AddDefinition(new LoadUntaggedInstr(
413-
new Value(array), target::PointerBase::data_offset()));
413+
array = builder.AddDefinition(new LoadFieldInstr(
414+
new Value(array), Slot::PointerBase_data(),
415+
InnerPointerAccess::kCannotBeInnerPointer, builder.Source()));
414416
}
415417
// No store barrier.
416418
ASSERT(IsExternalTypedDataClassId(array_cid) ||
@@ -547,11 +549,13 @@ static bool BuildCodeUnitAt(FlowGraph* flow_graph, intptr_t cid) {
547549

548550
// For external strings: Load external data.
549551
if (cid == kExternalOneByteStringCid) {
550-
str = builder.AddDefinition(new LoadUntaggedInstr(
551-
new Value(str), target::ExternalOneByteString::external_data_offset()));
552+
str = builder.AddDefinition(new LoadFieldInstr(
553+
new Value(str), Slot::ExternalOneByteString_external_data(),
554+
InnerPointerAccess::kCannotBeInnerPointer, builder.Source()));
552555
} else if (cid == kExternalTwoByteStringCid) {
553-
str = builder.AddDefinition(new LoadUntaggedInstr(
554-
new Value(str), target::ExternalTwoByteString::external_data_offset()));
556+
str = builder.AddDefinition(new LoadFieldInstr(
557+
new Value(str), Slot::ExternalTwoByteString_external_data(),
558+
InnerPointerAccess::kCannotBeInnerPointer, builder.Source()));
555559
}
556560

557561
Definition* load = builder.AddDefinition(new LoadIndexedInstr(

‎runtime/vm/regexp_assembler_ir.cc

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1725,17 +1725,19 @@ Value* IRRegExpMacroAssembler::LoadCodeUnitsAt(LocalVariable* index,
17251725
Value* pattern_val = BindLoadLocal(*string_param_);
17261726
if (IsExternalStringClassId(specialization_cid_)) {
17271727
// The data of an external string is stored through one indirection.
1728-
intptr_t external_offset = 0;
1728+
const Slot* slot = nullptr;
17291729
if (specialization_cid_ == kExternalOneByteStringCid) {
1730-
external_offset = ExternalOneByteString::external_data_offset();
1730+
slot = &Slot::ExternalOneByteString_external_data();
17311731
} else if (specialization_cid_ == kExternalTwoByteStringCid) {
1732-
external_offset = ExternalTwoByteString::external_data_offset();
1732+
slot = &Slot::ExternalTwoByteString_external_data();
17331733
} else {
17341734
UNREACHABLE();
17351735
}
17361736
// This pushes an untagged value on the stack which is immediately consumed
17371737
// by LoadCodeUnitsAtInstr below.
1738-
pattern_val = Bind(new (Z) LoadUntaggedInstr(pattern_val, external_offset));
1738+
pattern_val = Bind(new (Z) LoadFieldInstr(
1739+
pattern_val, *slot, InnerPointerAccess::kCannotBeInnerPointer,
1740+
InstructionSource()));
17391741
}
17401742

17411743
// Here pattern_val might be untagged so this must not trigger a GC.

0 commit comments

Comments
 (0)