Skip to content

Commit 1573f4e

Browse files
sstricklcommit-bot@chromium.org
sstrickl
authored and
[email protected]
committed
[vm/compiler] Further work on the IL deserializer.
Adds support for: CheckStackOverflow Parameter SpecialParameter Adds handling of Integer, Double, Class, Type, and TypeArguments in ParseDartValue. Adds handling of values visited in the graph prior to the corresponding definition to keep traversal of block contents as one pass. Results from compiling hello world program: * Early round trip: * Contains unhandled instructions: 4157 * Failed during deserialization: 0 * Successful round trips: 21 * Late round trip: * Contains unhandled instructions: 4023 * Failed during deserialization: 29 * Successful round trips: 126 Bug: #36882 Change-Id: Icb2972749ee86891d5847519a3b5097ad5c63c54 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/113326 Commit-Queue: Teagan Strickland <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
1 parent 45b85e1 commit 1573f4e

File tree

6 files changed

+203
-32
lines changed

6 files changed

+203
-32
lines changed

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4244,6 +4244,30 @@ void MaterializeObjectInstr::RemapRegisters(intptr_t* cpu_reg_slots,
42444244
}
42454245
}
42464246

4247+
const char* SpecialParameterInstr::KindToCString(SpecialParameterKind k) {
4248+
switch (k) {
4249+
#define KIND_CASE(Name) \
4250+
case SpecialParameterKind::k##Name: \
4251+
return #Name;
4252+
FOR_EACH_SPECIAL_PARAMETER_KIND(KIND_CASE)
4253+
#undef KIND_CASE
4254+
}
4255+
return nullptr;
4256+
}
4257+
4258+
bool SpecialParameterInstr::KindFromCString(const char* str,
4259+
SpecialParameterKind* out) {
4260+
ASSERT(str != nullptr && out != nullptr);
4261+
#define KIND_CASE(Name) \
4262+
if (strcmp(str, #Name) == 0) { \
4263+
*out = SpecialParameterKind::k##Name; \
4264+
return true; \
4265+
}
4266+
FOR_EACH_SPECIAL_PARAMETER_KIND(KIND_CASE)
4267+
#undef KIND_CASE
4268+
return false;
4269+
}
4270+
42474271
LocationSummary* SpecialParameterInstr::MakeLocationSummary(Zone* zone,
42484272
bool opt) const {
42494273
// Only appears in initial definitions, never in normal code.

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

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3278,13 +3278,25 @@ class AssertBooleanInstr : public TemplateDefinition<1, Throws, Pure> {
32783278
// the type arguments of a generic function or an arguments descriptor.
32793279
class SpecialParameterInstr : public TemplateDefinition<0, NoThrow> {
32803280
public:
3281-
enum SpecialParameterKind {
3282-
kContext,
3283-
kTypeArgs,
3284-
kArgDescriptor,
3285-
kException,
3286-
kStackTrace
3287-
};
3281+
#define FOR_EACH_SPECIAL_PARAMETER_KIND(M) \
3282+
M(Context) \
3283+
M(TypeArgs) \
3284+
M(ArgDescriptor) \
3285+
M(Exception) \
3286+
M(StackTrace)
3287+
3288+
#define KIND_DECL(name) k##name,
3289+
enum SpecialParameterKind { FOR_EACH_SPECIAL_PARAMETER_KIND(KIND_DECL) };
3290+
#undef KIND_DECL
3291+
3292+
// Defined as a static intptr_t instead of inside the enum since some
3293+
// switch statements depend on the exhaustibility checking.
3294+
#define KIND_INC(name) +1
3295+
static const intptr_t kNumKinds = 0 FOR_EACH_SPECIAL_PARAMETER_KIND(KIND_INC);
3296+
#undef KIND_INC
3297+
3298+
static const char* KindToCString(SpecialParameterKind k);
3299+
static bool KindFromCString(const char* str, SpecialParameterKind* out);
32883300

32893301
SpecialParameterInstr(SpecialParameterKind kind,
32903302
intptr_t deopt_id,
@@ -3311,23 +3323,6 @@ class SpecialParameterInstr : public TemplateDefinition<0, NoThrow> {
33113323
PRINT_OPERANDS_TO_SUPPORT
33123324
ADD_OPERANDS_TO_S_EXPRESSION_SUPPORT
33133325

3314-
static const char* KindToCString(SpecialParameterKind kind) {
3315-
switch (kind) {
3316-
case kContext:
3317-
return "kContext";
3318-
case kTypeArgs:
3319-
return "kTypeArgs";
3320-
case kArgDescriptor:
3321-
return "kArgDescriptor";
3322-
case kException:
3323-
return "kException";
3324-
case kStackTrace:
3325-
return "kStackTrace";
3326-
}
3327-
UNREACHABLE();
3328-
return NULL;
3329-
}
3330-
33313326
private:
33323327
const SpecialParameterKind kind_;
33333328
BlockEntryInstr* block_;

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

Lines changed: 130 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,12 @@ Instruction* FlowGraphDeserializer::FirstUnhandledInstruction(
126126
return nullptr;
127127
}
128128

129-
// Keep in sync with work in ParseDartValue.
129+
// Keep in sync with work in ParseDartValue. Right now, this is just a shallow
130+
// check, not a deep one.
130131
bool FlowGraphDeserializer::IsHandledConstant(const Object& obj) {
131-
return obj.IsNull() || obj.IsBool() || obj.IsString();
132+
return obj.IsNull() || obj.IsBool() || obj.IsString() || obj.IsInteger() ||
133+
obj.IsDouble() || obj.IsClass() || obj.IsType() ||
134+
obj.IsTypeArguments();
132135
}
133136

134137
SExpression* FlowGraphDeserializer::Retrieve(SExpList* list, intptr_t index) {
@@ -222,6 +225,16 @@ FlowGraph* FlowGraphDeserializer::ParseFlowGraph() {
222225
}
223226
}
224227

228+
// Before we return the new graph, make sure all definitions were found for
229+
// all pending values.
230+
if (values_map_.Length() > 0) {
231+
auto it = values_map_.GetIterator();
232+
auto const kv = it.Next();
233+
StoreError(new (zone()) SExpInteger(kv->key),
234+
"no definition found for variable index in flow graph");
235+
return nullptr;
236+
}
237+
225238
flow_graph_->set_max_block_id(max_block_id_);
226239
flow_graph_->set_current_ssa_temp_index(max_ssa_index_ + 1);
227240
// Now that the deserializer has finished re-creating all the blocks in the
@@ -406,6 +419,7 @@ bool FlowGraphDeserializer::ParseDefinitionWithParsedBody(SExpList* list,
406419
}
407420

408421
definition_map_.Insert(index, def);
422+
FixPendingValues(index, def);
409423
return true;
410424
}
411425

@@ -460,13 +474,63 @@ Instruction* FlowGraphDeserializer::ParseInstruction(SExpList* list) {
460474
return inst;
461475
}
462476

477+
CheckStackOverflowInstr* FlowGraphDeserializer::HandleCheckStackOverflow(
478+
SExpList* sexp,
479+
const CommonInstrInfo& info) {
480+
intptr_t stack_depth = 0;
481+
if (auto const stack_sexp =
482+
CheckInteger(sexp->ExtraLookupValue("stack_depth"))) {
483+
stack_depth = stack_sexp->value();
484+
}
485+
486+
intptr_t loop_depth = 0;
487+
if (auto const loop_sexp =
488+
CheckInteger(sexp->ExtraLookupValue("loop_depth"))) {
489+
loop_depth = loop_sexp->value();
490+
}
491+
492+
auto kind = CheckStackOverflowInstr::kOsrAndPreemption;
493+
if (auto const kind_sexp = CheckSymbol(sexp->ExtraLookupValue("kind"))) {
494+
ASSERT(strcmp(kind_sexp->value(), "OsrOnly") == 0);
495+
kind = CheckStackOverflowInstr::kOsrOnly;
496+
}
497+
498+
return new (zone()) CheckStackOverflowInstr(info.token_pos, stack_depth,
499+
loop_depth, info.deopt_id, kind);
500+
}
501+
502+
ParameterInstr* FlowGraphDeserializer::HandleParameter(
503+
SExpList* sexp,
504+
const CommonInstrInfo& info) {
505+
ASSERT(current_block_ != nullptr);
506+
if (auto const index_sexp = CheckInteger(Retrieve(sexp, 1))) {
507+
return new (zone()) ParameterInstr(index_sexp->value(), current_block_);
508+
}
509+
return nullptr;
510+
}
511+
463512
ReturnInstr* FlowGraphDeserializer::HandleReturn(SExpList* list,
464513
const CommonInstrInfo& info) {
465514
Value* val = ParseValue(Retrieve(list, 1));
466515
if (val == nullptr) return nullptr;
467516
return new (zone()) ReturnInstr(info.token_pos, val, info.deopt_id);
468517
}
469518

519+
SpecialParameterInstr* FlowGraphDeserializer::HandleSpecialParameter(
520+
SExpList* sexp,
521+
const CommonInstrInfo& info) {
522+
ASSERT(current_block_ != nullptr);
523+
auto const kind_sexp = CheckSymbol(Retrieve(sexp, 1));
524+
if (kind_sexp == nullptr) return nullptr;
525+
SpecialParameterInstr::SpecialParameterKind kind;
526+
if (!SpecialParameterInstr::KindFromCString(kind_sexp->value(), &kind)) {
527+
StoreError(kind_sexp, "unknown special parameter kind");
528+
return nullptr;
529+
}
530+
return new (zone())
531+
SpecialParameterInstr(kind, info.deopt_id, current_block_);
532+
}
533+
470534
Value* FlowGraphDeserializer::ParseValue(SExpression* sexp) {
471535
auto name = sexp->AsSymbol();
472536
CompileType* type = nullptr;
@@ -485,9 +549,7 @@ Value* FlowGraphDeserializer::ParseValue(SExpression* sexp) {
485549
auto const def = definition_map_.LookupValue(index);
486550
Value* val;
487551
if (def == nullptr) {
488-
// TODO(sstrickl): Handle uses that come before parsed definitions.
489-
StoreError(name, "use found before definition");
490-
return nullptr;
552+
val = AddPendingValue(index);
491553
} else {
492554
val = new (zone()) Value(def);
493555
}
@@ -581,10 +643,47 @@ bool FlowGraphDeserializer::ParseDartValue(SExpression* sexp, Object* out) {
581643

582644
// Other instance values may need to be canonicalized, so do that before
583645
// returning.
584-
if (auto const b = sexp->AsBool()) {
646+
if (auto const list = CheckTaggedList(sexp)) {
647+
auto const tag = list->At(0)->AsSymbol();
648+
if (strcmp(tag->value(), "Class") == 0) {
649+
auto const cid_sexp = CheckInteger(Retrieve(list, 1));
650+
if (cid_sexp == nullptr) return false;
651+
ClassTable* table = thread()->isolate()->class_table();
652+
if (!table->IsValidIndex(cid_sexp->value())) {
653+
StoreError(cid_sexp, "no class found for cid");
654+
return false;
655+
}
656+
*out = table->At(cid_sexp->value());
657+
} else if (strcmp(tag->value(), "Type") == 0) {
658+
if (const auto cls_sexp = CheckTaggedList(Retrieve(list, 1), "Class")) {
659+
auto& cls = Class::ZoneHandle(zone());
660+
if (!ParseDartValue(cls_sexp, &cls)) return false;
661+
auto& type_args = TypeArguments::ZoneHandle(zone());
662+
if (const auto ta_sexp = CheckTaggedList(
663+
list->ExtraLookupValue("type_args"), "TypeArguments")) {
664+
if (!ParseDartValue(ta_sexp, &type_args)) return false;
665+
}
666+
*out = Type::New(cls, type_args, TokenPosition::kNoSource, Heap::kOld);
667+
// Need to set this for canonicalization.
668+
Type::Cast(*out).SetIsFinalized();
669+
}
670+
// TODO(sstrickl): Handle types not derived from classes.
671+
} else if (strcmp(tag->value(), "TypeArguments") == 0) {
672+
*out = TypeArguments::New(list->Length() - 1, Heap::kOld);
673+
auto& typ = AbstractType::Handle(zone());
674+
for (intptr_t i = 1; i < list->Length(); i++) {
675+
if (!ParseDartValue(Retrieve(list, i), &typ)) return false;
676+
TypeArguments::Cast(*out).SetTypeAt(i - 1, typ);
677+
}
678+
}
679+
} else if (auto const b = sexp->AsBool()) {
585680
*out = Bool::Get(b->value()).raw();
586681
} else if (auto const str = sexp->AsString()) {
587682
*out = String::New(str->value(), Heap::kOld);
683+
} else if (auto const i = sexp->AsInteger()) {
684+
*out = Integer::New(i->value(), Heap::kOld);
685+
} else if (auto const d = sexp->AsDouble()) {
686+
*out = Double::New(d->value(), Heap::kOld);
588687
}
589688

590689
// If we're here and still haven't gotten a non-null value, then something
@@ -645,6 +744,29 @@ bool FlowGraphDeserializer::ParseSymbolAsPrefixedInt(SExpSymbol* sym,
645744
return true;
646745
}
647746

747+
Value* FlowGraphDeserializer::AddPendingValue(intptr_t index) {
748+
ASSERT(flow_graph_ != nullptr);
749+
ASSERT(!definition_map_.HasKey(index));
750+
auto value_list = values_map_.LookupValue(index);
751+
if (value_list == nullptr) {
752+
value_list = new (zone()) ZoneGrowableArray<Value*>(zone(), 2);
753+
values_map_.Insert(index, value_list);
754+
}
755+
auto const val = new (zone()) Value(flow_graph_->constant_null());
756+
value_list->Add(val);
757+
return val;
758+
}
759+
760+
void FlowGraphDeserializer::FixPendingValues(intptr_t index, Definition* def) {
761+
if (auto value_list = values_map_.LookupValue(index)) {
762+
for (intptr_t i = 0; i < value_list->length(); i++) {
763+
auto const val = value_list->At(i);
764+
val->BindTo(def);
765+
}
766+
values_map_.Remove(index);
767+
}
768+
}
769+
648770
#define BASE_CHECK_DEF(name, type) \
649771
SExp##name* FlowGraphDeserializer::Check##name(SExpression* sexp) { \
650772
if (sexp == nullptr) return nullptr; \
@@ -657,6 +779,8 @@ bool FlowGraphDeserializer::ParseSymbolAsPrefixedInt(SExpSymbol* sym,
657779

658780
FOR_EACH_S_EXPRESSION(BASE_CHECK_DEF)
659781

782+
#undef BASE_CHECK_DEF
783+
660784
bool FlowGraphDeserializer::IsTag(SExpression* sexp, const char* label) {
661785
auto const sym = CheckSymbol(sexp);
662786
if (sym == nullptr) return false;

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ class FlowGraphDeserializer : ValueObject {
4646
root_sexp_(ASSERT_NOTNULL(root)),
4747
parsed_function_(pf),
4848
block_map_(zone_),
49-
definition_map_(zone_) {
49+
definition_map_(zone_),
50+
values_map_(zone_) {
5051
// See canonicalization comment in ParseDartValue as to why this is
5152
// currently necessary.
5253
ASSERT(thread->zone() == zone);
@@ -67,7 +68,11 @@ class FlowGraphDeserializer : ValueObject {
6768
M(GraphEntry) \
6869
M(TargetEntry)
6970

70-
#define FOR_EACH_HANDLED_INSTRUCTION_IN_DESERIALIZER(M) M(Return)
71+
#define FOR_EACH_HANDLED_INSTRUCTION_IN_DESERIALIZER(M) \
72+
M(CheckStackOverflow) \
73+
M(Parameter) \
74+
M(Return) \
75+
M(SpecialParameter)
7176

7277
// Helper method for FirstUnhandledInstruction that returns whether a given
7378
// object should be (de)serializable. Any work done on ParseDartValue may
@@ -162,6 +167,14 @@ class FlowGraphDeserializer : ValueObject {
162167
bool ParseUse(SExpSymbol* sym, intptr_t* out);
163168
bool ParseSymbolAsPrefixedInt(SExpSymbol* sym, char prefix, intptr_t* out);
164169

170+
// Helper function for creating a placeholder value when the definition
171+
// has not yet been seen.
172+
Value* AddPendingValue(intptr_t index);
173+
174+
// Helper function for rebinding pending values once the definition has
175+
// been located.
176+
void FixPendingValues(intptr_t index, Definition* def);
177+
165178
// Utility functions for checking the shape of an S-expression.
166179
// If these functions return nullptr for a non-null argument, they have the
167180
// side effect of setting the stored error message.
@@ -202,6 +215,10 @@ class FlowGraphDeserializer : ValueObject {
202215
// Map from variable indexes to definitions.
203216
IntMap<Definition*> definition_map_;
204217

218+
// Map from variable indices to lists of values. The list of values are
219+
// values that were parsed prior to the corresponding definition being found.
220+
IntMap<ZoneGrowableArray<Value*>*> values_map_;
221+
205222
// Stores a message appropriate to surfacing to the user when an error
206223
// occurs.
207224
const char* error_message_ = nullptr;

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,7 @@ SExpression* FlowGraphSerializer::AbstractTypeToSExp(const AbstractType& t) {
362362
ASSERT(t.IsType());
363363
AddSymbol(sexp, "Type");
364364
const auto& typ = Type::Cast(t);
365+
ASSERT(typ.IsFinalized());
365366
if (typ.HasTypeClass()) {
366367
type_class_ = typ.type_class();
367368
// This avoids re-entry as long as serializing a class doesn't involve
@@ -728,6 +729,7 @@ void ParameterInstr::AddOperandsToSExpression(SExpList* sexp,
728729
void SpecialParameterInstr::AddOperandsToSExpression(
729730
SExpList* sexp,
730731
FlowGraphSerializer* s) const {
732+
ASSERT(kind() < SpecialParameterInstr::kNumKinds);
731733
s->AddSymbol(sexp, KindToCString(kind()));
732734
}
733735

@@ -1022,6 +1024,10 @@ void CheckStackOverflowInstr::AddExtraInfoToSExpression(
10221024
if (in_loop() || FLAG_verbose_flow_graph_serialization) {
10231025
s->AddExtraInteger(sexp, "loop_depth", loop_depth());
10241026
}
1027+
if (kind_ != kOsrAndPreemption) {
1028+
ASSERT(kind_ == kOsrOnly);
1029+
s->AddExtraSymbol(sexp, "kind", "OsrOnly");
1030+
}
10251031
}
10261032

10271033
SExpression* Value::ToSExpression(FlowGraphSerializer* s) const {

‎runtime/vm/object.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6648,6 +6648,11 @@ class TypeArguments : public Instance {
66486648
// Return true if this vector contains a recursive type argument.
66496649
bool IsRecursive() const;
66506650

6651+
virtual RawInstance* CheckAndCanonicalize(Thread* thread,
6652+
const char** error_str) const {
6653+
return Canonicalize();
6654+
}
6655+
66516656
// Canonicalize only if instantiated, otherwise returns 'this'.
66526657
RawTypeArguments* Canonicalize(TrailPtr trail = NULL) const;
66536658

0 commit comments

Comments
 (0)