Skip to content

Commit f255c0c

Browse files
alexmarkovcommit-bot@chromium.org
alexmarkov
authored and
[email protected]
committed
[vm/bytecode] Revise PushStatic bytecode instruction
PushStatic instruction expects field object to be pushed onto the stack. This is useless, as PushStatic also references field with its D operand. This change replaces PushStatic instruction with LoadStatic which doesn't take field object on the stack, so PushConstant/PushStatic pair is replaced with a single LoadStatic instruction. This change also enables constant propagation of values of injected CID fields, which are not known at bytecode generation time. Change-Id: Ifbdd3aea2aab338f6c9ec3e5728948c4d8541a4b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112489 Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Ryan Macnak <[email protected]> Reviewed-by: Régis Crelier <[email protected]>
1 parent f74b0cc commit f255c0c

File tree

9 files changed

+62
-48
lines changed

9 files changed

+62
-48
lines changed

‎pkg/vm/lib/bytecode/assembler.dart‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -404,15 +404,15 @@ class BytecodeAssembler {
404404
_emitInstructionD(Opcode.kNativeCall, rd);
405405
}
406406

407+
void emitLoadStatic(int rd) {
408+
_emitInstructionD(Opcode.kLoadStatic, rd);
409+
}
410+
407411
void emitStoreStaticTOS(int rd) {
408412
emitSourcePosition();
409413
_emitInstructionD(Opcode.kStoreStaticTOS, rd);
410414
}
411415

412-
void emitPushStatic(int rd) {
413-
_emitInstructionD(Opcode.kPushStatic, rd);
414-
}
415-
416416
void emitCreateArrayTOS() {
417417
_emitInstruction0(Opcode.kCreateArrayTOS);
418418
}

‎pkg/vm/lib/bytecode/dbc.dart‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ library vm.bytecode.dbc;
1010
/// Before bumping current bytecode version format, make sure that
1111
/// all users have switched to a VM which is able to consume new
1212
/// version of bytecode.
13-
const int currentBytecodeFormatVersion = 18;
13+
const int currentBytecodeFormatVersion = 19;
1414

1515
enum Opcode {
1616
kUnusedOpcode000,
@@ -168,8 +168,8 @@ enum Opcode {
168168
kUnused17, // Reserved for PushParamLast1
169169
kPopLocal,
170170
kPopLocal_Wide,
171-
kUnused18, // Reserved for PopLocal0
172-
kUnused19,
171+
kLoadStatic,
172+
kLoadStatic_Wide,
173173
kStoreLocal,
174174
kStoreLocal_Wide,
175175

@@ -182,8 +182,8 @@ enum Opcode {
182182
kUnused20,
183183

184184
// Static fields.
185-
kPushStatic,
186-
kPushStatic_Wide,
185+
kUnused40,
186+
kUnused41,
187187
kStoreStaticTOS,
188188
kStoreStaticTOS_Wide,
189189

@@ -412,7 +412,7 @@ const Map<Opcode, Format> BytecodeFormats = const {
412412
Encoding.kD, const [Operand.lit, Operand.none, Operand.none]),
413413
Opcode.kStoreIndexedTOS: const Format(
414414
Encoding.k0, const [Operand.none, Operand.none, Operand.none]),
415-
Opcode.kPushStatic: const Format(
415+
Opcode.kLoadStatic: const Format(
416416
Encoding.kD, const [Operand.lit, Operand.none, Operand.none]),
417417
Opcode.kStoreStaticTOS: const Format(
418418
Encoding.kD, const [Operand.lit, Operand.none, Operand.none]),
@@ -633,7 +633,6 @@ bool isPush(Opcode opcode) {
633633
case Opcode.kPushTrue:
634634
case Opcode.kPushFalse:
635635
case Opcode.kPushInt:
636-
case Opcode.kPushStatic:
637636
return true;
638637
default:
639638
return false;

‎pkg/vm/lib/bytecode/gen_bytecode.dart‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3074,10 +3074,7 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
30743074
if (target.isConst) {
30753075
_genPushConstExpr(target.initializer);
30763076
} else if (_hasTrivialInitializer(target)) {
3077-
final fieldIndex = cp.addStaticField(target);
3078-
asm.emitPushConstant(
3079-
fieldIndex); // TODO(alexmarkov): do we really need this?
3080-
asm.emitPushStatic(fieldIndex);
3077+
asm.emitLoadStatic(cp.addStaticField(target));
30813078
} else {
30823079
_genDirectCall(target, objectTable.getArgDescHandle(0), 0, isGet: true);
30833080
}

‎pkg/vm/testcases/bytecode/bootstrapping.dart.expect‎

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -124,14 +124,12 @@ Function '_scriptUri', static, reflectable, debuggable
124124
Bytecode {
125125
Entry 2
126126
CheckStack 0
127-
PushConstant CP#0
128-
PushStatic CP#0
127+
LoadStatic CP#0
129128
PushConstant CP#1
130129
InterfaceCall CP#2, 2
131130
AssertBoolean 0
132131
JumpIfTrue L1
133-
PushConstant CP#0
134-
PushStatic CP#0
132+
LoadStatic CP#0
135133
PushConstant CP#4
136134
InterfaceCall CP#2, 2
137135
AssertBoolean 0
@@ -143,8 +141,7 @@ L1:
143141
L2:
144142
Push r1
145143
JumpIfTrue L3
146-
PushConstant CP#0
147-
PushStatic CP#0
144+
LoadStatic CP#0
148145
PushConstant CP#5
149146
InterfaceCall CP#2, 2
150147
AssertBoolean 0
@@ -156,15 +153,13 @@ L3:
156153
L4:
157154
Push r0
158155
JumpIfFalse L5
159-
PushConstant CP#0
160-
PushStatic CP#0
156+
LoadStatic CP#0
161157
DirectCall CP#6, 1
162158
ReturnTOS
163159
L5:
164160
DirectCall CP#8, 0
165161
PushNull
166-
PushConstant CP#0
167-
PushStatic CP#0
162+
LoadStatic CP#0
168163
DirectCall CP#10, 2
169164
InterfaceCall CP#12, 2
170165
ReturnTOS
@@ -214,12 +209,10 @@ Function 'get:stdin', getter, static, reflectable, debuggable
214209
Bytecode {
215210
Entry 2
216211
CheckStack 0
217-
PushConstant CP#0
218-
PushStatic CP#0
212+
LoadStatic CP#0
219213
EqualsNull
220214
JumpIfFalse L1
221-
PushConstant CP#1
222-
PushStatic CP#1
215+
LoadStatic CP#1
223216
DirectCall CP#2, 1
224217
StoreLocal r1
225218
Push r1
@@ -232,8 +225,7 @@ L1:
232225
L2:
233226
Push r0
234227
Drop1
235-
PushConstant CP#0
236-
PushStatic CP#0
228+
LoadStatic CP#0
237229
ReturnTOS
238230
}
239231
ConstantPool {
@@ -390,8 +382,7 @@ Function 'get:_namespace', getter, static, reflectable, debuggable
390382
Bytecode {
391383
Entry 2
392384
CheckStack 0
393-
PushConstant CP#0
394-
PushStatic CP#0
385+
LoadStatic CP#0
395386
EqualsNull
396387
JumpIfFalse L1
397388
Allocate CP#1
@@ -403,8 +394,7 @@ Bytecode {
403394
DirectCall CP#6, 2
404395
StoreStaticTOS CP#0
405396
L1:
406-
PushConstant CP#0
407-
PushStatic CP#0
397+
LoadStatic CP#0
408398
ReturnTOS
409399
}
410400
ConstantPool {
@@ -592,12 +582,10 @@ Function 'get:platformScript', getter, static, reflectable, debuggable
592582
Bytecode {
593583
Entry 1
594584
CheckStack 0
595-
PushConstant CP#0
596-
PushStatic CP#0
585+
LoadStatic CP#0
597586
EqualsNull
598587
JumpIfFalse L1
599-
PushConstant CP#1
600-
PushStatic CP#1
588+
LoadStatic CP#1
601589
EqualsNull
602590
BooleanNegateTOS
603591
PopLocal r0
@@ -608,13 +596,11 @@ L1:
608596
L2:
609597
Push r0
610598
JumpIfFalse L3
611-
PushConstant CP#1
612-
PushStatic CP#1
599+
LoadStatic CP#1
613600
DynamicCall CP#3, 1
614601
StoreStaticTOS CP#0
615602
L3:
616-
PushConstant CP#0
617-
PushStatic CP#0
603+
LoadStatic CP#0
618604
ReturnTOS
619605
}
620606
ConstantPool {

‎pkg/vm/testcases/bytecode/literals.dart.expect‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -390,8 +390,7 @@ Function 'testFieldWithDoubleLiteralInitializer', static, reflectable, debuggabl
390390
Bytecode {
391391
Entry 0
392392
CheckStack 0
393-
PushConstant CP#0
394-
PushStatic CP#0
393+
LoadStatic CP#0
395394
ReturnTOS
396395
}
397396
ConstantPool {

‎runtime/vm/compiler/assembler/disassembler_kbc.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,8 @@ static intptr_t GetConstantPoolIndex(const KBCInstr* instr) {
247247
case KernelBytecode::kPushConstant_Wide:
248248
case KernelBytecode::kStoreStaticTOS:
249249
case KernelBytecode::kStoreStaticTOS_Wide:
250+
case KernelBytecode::kLoadStatic:
251+
case KernelBytecode::kLoadStatic_Wide:
250252
case KernelBytecode::kPushStatic:
251253
case KernelBytecode::kPushStatic_Wide:
252254
case KernelBytecode::kAllocate:

‎runtime/vm/compiler/frontend/bytecode_flow_graph_builder.cc‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,6 +1207,25 @@ void BytecodeFlowGraphBuilder::BuildStoreStaticTOS() {
12071207
code_ += B->StoreStaticField(position_, field);
12081208
}
12091209

1210+
void BytecodeFlowGraphBuilder::BuildLoadStatic() {
1211+
const Constant operand = ConstantAt(DecodeOperandD());
1212+
const auto& field = Field::Cast(operand.value());
1213+
// All constant expressions (including access to const fields) are evaluated
1214+
// in bytecode. However, values of injected cid fields are only available in
1215+
// the VM. In such case, evaluate const fields with known value here.
1216+
if (field.is_const() && !field.has_initializer()) {
1217+
const auto& value = Object::ZoneHandle(Z, field.StaticValue());
1218+
ASSERT((value.raw() != Object::sentinel().raw()) &&
1219+
(value.raw() != Object::transition_sentinel().raw()));
1220+
code_ += B->Constant(value);
1221+
return;
1222+
}
1223+
PushConstant(operand);
1224+
code_ += B->LoadStaticField();
1225+
}
1226+
1227+
static_assert(KernelBytecode::kMinSupportedBytecodeFormatVersion < 19,
1228+
"Cleanup PushStatic bytecode instruction");
12101229
void BytecodeFlowGraphBuilder::BuildPushStatic() {
12111230
// Note: Field object is both pushed into the stack and
12121231
// available in constant pool entry D.

‎runtime/vm/constants_kbc.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -615,8 +615,8 @@ namespace dart {
615615
V(Unused17, 0, RESV, ___, ___, ___) \
616616
V(PopLocal, X, ORDN, xeg, ___, ___) \
617617
V(PopLocal_Wide, X, WIDE, xeg, ___, ___) \
618-
V(Unused18, 0, RESV, ___, ___, ___) \
619-
V(Unused19, 0, RESV, ___, ___, ___) \
618+
V(LoadStatic, D, ORDN, lit, ___, ___) \
619+
V(LoadStatic_Wide, D, WIDE, lit, ___, ___) \
620620
V(StoreLocal, X, ORDN, xeg, ___, ___) \
621621
V(StoreLocal_Wide, X, WIDE, xeg, ___, ___) \
622622
V(LoadFieldTOS, D, ORDN, lit, ___, ___) \
@@ -749,7 +749,7 @@ class KernelBytecode {
749749
// Maximum bytecode format version supported by VM.
750750
// The range of supported versions should include version produced by bytecode
751751
// generator (currentBytecodeFormatVersion in pkg/vm/lib/bytecode/dbc.dart).
752-
static const intptr_t kMaxSupportedBytecodeFormatVersion = 18;
752+
static const intptr_t kMaxSupportedBytecodeFormatVersion = 19;
753753

754754
enum Opcode {
755755
#define DECLARE_BYTECODE(name, encoding, kind, op1, op2, op3) k##name,

‎runtime/vm/interpreter.cc‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2362,13 +2362,25 @@ RawObject* Interpreter::Call(RawFunction* function,
23622362
}
23632363

23642364
{
2365+
static_assert(KernelBytecode::kMinSupportedBytecodeFormatVersion < 19,
2366+
"Cleanup PushStatic bytecode instruction");
23652367
BYTECODE(PushStatic, D);
23662368
RawField* field = reinterpret_cast<RawField*>(LOAD_CONSTANT(rD));
23672369
// Note: field is also on the stack, hence no increment.
23682370
*SP = field->ptr()->value_.static_value_;
23692371
DISPATCH();
23702372
}
23712373

2374+
{
2375+
BYTECODE(LoadStatic, D);
2376+
RawField* field = reinterpret_cast<RawField*>(LOAD_CONSTANT(rD));
2377+
RawInstance* value = field->ptr()->value_.static_value_;
2378+
ASSERT((value != Object::sentinel().raw()) &&
2379+
(value != Object::transition_sentinel().raw()));
2380+
*++SP = value;
2381+
DISPATCH();
2382+
}
2383+
23722384
{
23732385
BYTECODE(StoreFieldTOS, D);
23742386
RawField* field = RAW_CAST(Field, LOAD_CONSTANT(rD + 1));

0 commit comments

Comments
 (0)