Skip to content

Commit 7929e1c

Browse files
sjindel-googlecommit-bot@chromium.org
sjindel-google
authored and
commit-bot@chromium.org
committed
[vm/ffi] Remove LocalVariable references from Allocate/CloneContext instructions
Also some other cleanups from https://dart-review.googlesource.com/c/sdk/+/107407, and exclude scopes.cc from the precompiled runtime. Change-Id: If14e46d44e6114a123a1eae3c85b63519b2863ac Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/108274 Commit-Queue: Samir Jindel <sjindel@google.com> Reviewed-by: Martin Kustermann <kustermann@google.com>
1 parent 359c36a commit 7929e1c

17 files changed

+86
-83
lines changed

‎pkg/test_runner/lib/src/command.dart‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -632,22 +632,14 @@ class AdbPrecompilationCommand extends Command implements AdbCommand {
632632
extraLibraries.forEach(builder.add);
633633
}
634634

635-
static bool _listEquals(List<String> x, List<String> y) {
636-
if (x.length != y.length) return false;
637-
for (int i = 0; i < x.length; ++i) {
638-
if (x[i] != y[i]) return false;
639-
}
640-
return true;
641-
}
642-
643635
bool _equal(AdbPrecompilationCommand other) =>
644636
super._equal(other) &&
645637
buildPath == other.buildPath &&
646638
useBlobs == other.useBlobs &&
647639
useElf == other.useElf &&
648640
arguments == other.arguments &&
649641
precompiledTestDirectory == other.precompiledTestDirectory &&
650-
_listEquals(extraLibraries, other.extraLibraries);
642+
deepJsonCompare(extraLibraries, other.extraLibraries);
651643

652644
String toString() => 'Steps to push precompiled runner and precompiled code '
653645
'to an attached device. Uses (and requires) adb.';

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

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5562,27 +5562,25 @@ class InstantiateTypeArgumentsInstr : public TemplateDefinition<2, Throws> {
55625562
class AllocateContextInstr : public TemplateAllocation<0, NoThrow> {
55635563
public:
55645564
AllocateContextInstr(TokenPosition token_pos,
5565-
const GrowableArray<LocalVariable*>& context_variables)
5566-
: token_pos_(token_pos), context_variables_(context_variables) {}
5565+
const ZoneGrowableArray<const Slot*>& context_slots)
5566+
: token_pos_(token_pos), context_slots_(context_slots) {}
55675567

55685568
DECLARE_INSTRUCTION(AllocateContext)
55695569
virtual CompileType ComputeType() const;
55705570

55715571
virtual TokenPosition token_pos() const { return token_pos_; }
5572-
const GrowableArray<LocalVariable*>& context_variables() const {
5573-
return context_variables_;
5572+
const ZoneGrowableArray<const Slot*>& context_slots() const {
5573+
return context_slots_;
55745574
}
55755575

5576-
intptr_t num_context_variables() const {
5577-
return context_variables().length();
5578-
}
5576+
intptr_t num_context_variables() const { return context_slots().length(); }
55795577

55805578
virtual bool ComputeCanDeoptimize() const { return false; }
55815579

55825580
virtual bool HasUnknownSideEffects() const { return false; }
55835581

55845582
virtual bool WillAllocateNewOrRemembered() const {
5585-
return WillAllocateNewOrRemembered(context_variables().length());
5583+
return WillAllocateNewOrRemembered(context_slots().length());
55865584
}
55875585

55885586
static bool WillAllocateNewOrRemembered(intptr_t num_context_variables) {
@@ -5595,7 +5593,7 @@ class AllocateContextInstr : public TemplateAllocation<0, NoThrow> {
55955593

55965594
private:
55975595
const TokenPosition token_pos_;
5598-
const GrowableArray<LocalVariable*>& context_variables_;
5596+
const ZoneGrowableArray<const Slot*>& context_slots_;
55995597

56005598
DISALLOW_COPY_AND_ASSIGN(AllocateContextInstr);
56015599
};
@@ -5629,19 +5627,19 @@ class CloneContextInstr : public TemplateDefinition<1, NoThrow> {
56295627
public:
56305628
CloneContextInstr(TokenPosition token_pos,
56315629
Value* context_value,
5632-
const GrowableArray<LocalVariable*>& context_variables,
5630+
const ZoneGrowableArray<const Slot*>& context_slots,
56335631
intptr_t deopt_id)
56345632
: TemplateDefinition(deopt_id),
56355633
token_pos_(token_pos),
5636-
context_variables_(context_variables) {
5634+
context_slots_(context_slots) {
56375635
SetInputAt(0, context_value);
56385636
}
56395637

56405638
virtual TokenPosition token_pos() const { return token_pos_; }
56415639
Value* context_value() const { return inputs_[0]; }
56425640

5643-
const GrowableArray<LocalVariable*>& context_variables() const {
5644-
return context_variables_;
5641+
const ZoneGrowableArray<const Slot*>& context_slots() const {
5642+
return context_slots_;
56455643
}
56465644

56475645
DECLARE_INSTRUCTION(CloneContext)
@@ -5653,7 +5651,7 @@ class CloneContextInstr : public TemplateDefinition<1, NoThrow> {
56535651

56545652
private:
56555653
const TokenPosition token_pos_;
5656-
const GrowableArray<LocalVariable*>& context_variables_;
5654+
const ZoneGrowableArray<const Slot*>& context_slots_;
56575655

56585656
DISALLOW_COPY_AND_ASSIGN(CloneContextInstr);
56595657
};

‎runtime/vm/compiler/compiler_state.cc‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// BSD-style license that can be found in the LICENSE file.
44

55
#include "vm/compiler/compiler_state.h"
6+
#include "vm/growable_array.h"
67

78
#ifndef DART_PRECOMPILED_RUNTIME
89

@@ -52,27 +53,22 @@ LocalVariable* CompilerState::GetDummyCapturedVariable(intptr_t context_id,
5253
});
5354
}
5455

55-
const GrowableArray<LocalVariable*>& CompilerState::GetDummyContextVariables(
56+
const ZoneGrowableArray<const Slot*>& CompilerState::GetDummyContextSlots(
5657
intptr_t context_id,
5758
intptr_t num_context_variables) {
58-
return PutIfAbsent<LocalScope>(
59-
thread(), &dummy_scopes_, num_context_variables,
60-
[&]() {
61-
Zone* const Z = thread()->zone();
62-
63-
LocalScope* scope = new (Z) LocalScope(
64-
/*parent=*/NULL, /*function_level=*/0, /*loop_level=*/0);
65-
scope->set_context_level(0);
59+
return *PutIfAbsent<ZoneGrowableArray<const Slot*>>(
60+
thread(), &dummy_slots_, num_context_variables, [&]() {
61+
Zone* const Z = thread()->zone();
6662

67-
for (intptr_t i = 0; i < num_context_variables; i++) {
68-
LocalVariable* var = GetDummyCapturedVariable(context_id, i);
69-
scope->AddVariable(var);
70-
scope->AddContextVariable(var);
71-
}
63+
auto slots =
64+
new (Z) ZoneGrowableArray<const Slot*>(num_context_variables);
65+
for (intptr_t i = 0; i < num_context_variables; i++) {
66+
LocalVariable* var = GetDummyCapturedVariable(context_id, i);
67+
slots->Add(&Slot::GetContextVariableSlotFor(thread(), *var));
68+
}
7269

73-
return scope;
74-
})
75-
->context_variables();
70+
return slots;
71+
});
7672
}
7773

7874
} // namespace dart

‎runtime/vm/compiler/compiler_state.h‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ namespace dart {
1414
class LocalScope;
1515
class LocalVariable;
1616
class SlotCache;
17+
class Slot;
1718

1819
// Deoptimization Id logic.
1920
//
@@ -97,9 +98,9 @@ class CompilerState : public ThreadStackResource {
9798
//
9899
// TODO(vegorov): create context classes for distinct context IDs and
99100
// populate them with slots without creating variables.
100-
const GrowableArray<LocalVariable*>& GetDummyContextVariables(
101+
const ZoneGrowableArray<const Slot*>& GetDummyContextSlots(
101102
intptr_t context_id,
102-
intptr_t num_context_variables);
103+
intptr_t num_context_slots);
103104

104105
// Create a dummy LocalVariable that represents a captured local variable
105106
// at the given index in the context with given ID.
@@ -121,9 +122,9 @@ class CompilerState : public ThreadStackResource {
121122
// Cache for Slot objects created during compilation (see slot.h).
122123
SlotCache* slot_cache_ = nullptr;
123124

124-
// Caches for dummy LocalVariables and LocalScopes created during
125-
// bytecode to IL translation.
126-
ZoneGrowableArray<LocalScope*>* dummy_scopes_ = nullptr;
125+
// Caches for dummy LocalVariables and context Slots created during bytecode
126+
// to IL translation.
127+
ZoneGrowableArray<ZoneGrowableArray<const Slot*>*>* dummy_slots_ = nullptr;
127128
ZoneGrowableArray<LocalVariable*>* dummy_captured_vars_ = nullptr;
128129

129130
CompilerState* previous_;

‎runtime/vm/compiler/ffi.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ bool IsAsFunctionInternal(Zone* zone, Isolate* isolate, const Function& func) {
578578
Object::Handle(zone, isolate->object_store()->ffi_as_function_internal());
579579
if (asFunctionInternal.raw() == Object::null()) {
580580
// Cache the reference.
581-
Library& ffi =
581+
const Library& ffi =
582582
Library::Handle(zone, isolate->object_store()->ffi_library());
583583
asFunctionInternal =
584584
ffi.LookupFunctionAllowPrivate(Symbols::AsFunctionInternal());

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include "vm/compiler/frontend/flow_graph_builder.h" // For InlineExitCollector.
88
#include "vm/compiler/jit/compiler.h" // For Compiler::IsBackgroundCompilation().
99
#include "vm/compiler/runtime_api.h"
10+
#include "vm/growable_array.h"
1011
#include "vm/object_store.h"
1112

1213
#if !defined(DART_PRECOMPILED_RUNTIME)
@@ -761,9 +762,9 @@ Fragment BaseFlowGraphBuilder::BooleanNegate() {
761762
}
762763

763764
Fragment BaseFlowGraphBuilder::AllocateContext(
764-
const GrowableArray<LocalVariable*>& context_variables) {
765+
const ZoneGrowableArray<const Slot*>& context_slots) {
765766
AllocateContextInstr* allocate =
766-
new (Z) AllocateContextInstr(TokenPosition::kNoSource, context_variables);
767+
new (Z) AllocateContextInstr(TokenPosition::kNoSource, context_slots);
767768
Push(allocate);
768769
return Fragment(allocate);
769770
}
@@ -845,16 +846,14 @@ Fragment BaseFlowGraphBuilder::BuildFfiAsFunctionInternalCall(
845846
code += LoadNativeField(Slot::Pointer_c_memory_address());
846847
LocalVariable* address = MakeTemporary();
847848

848-
auto& context_variables = CompilerState::Current().GetDummyContextVariables(
849+
auto& context_slots = CompilerState::Current().GetDummyContextSlots(
849850
/*context_id=*/0, /*num_variables=*/1);
850-
code += AllocateContext(context_variables);
851+
code += AllocateContext(context_slots);
851852
LocalVariable* context = MakeTemporary();
852853

853854
code += LoadLocal(context);
854855
code += LoadLocal(address);
855-
code += StoreInstanceField(
856-
TokenPosition::kNoSource,
857-
Slot::GetContextVariableSlotFor(thread_, *context_variables[0]));
856+
code += StoreInstanceField(TokenPosition::kNoSource, *context_slots[0]);
858857

859858
code += AllocateClosure(TokenPosition::kNoSource, target);
860859
LocalVariable* closure = MakeTemporary();

‎runtime/vm/compiler/frontend/base_flow_graph_builder.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ class BaseFlowGraphBuilder {
275275

276276
Fragment AssertBool(TokenPosition position);
277277
Fragment BooleanNegate();
278-
Fragment AllocateContext(const GrowableArray<LocalVariable*>& scope);
278+
Fragment AllocateContext(const ZoneGrowableArray<const Slot*>& scope);
279279
Fragment AllocateClosure(TokenPosition position,
280280
const Function& closure_function);
281281
Fragment CreateArray();

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -777,7 +777,7 @@ void BytecodeFlowGraphBuilder::BuildDirectCall() {
777777
const Function& target = Function::Cast(ConstantAt(DecodeOperandD()).value());
778778
const intptr_t argc = DecodeOperandF().value();
779779

780-
if (compiler::ffi::IsAsFunctionInternal(Z, Isolate::Current(), target)) {
780+
if (compiler::ffi::IsAsFunctionInternal(Z, isolate(), target)) {
781781
BuildFfiAsFunction();
782782
return;
783783
}
@@ -965,9 +965,9 @@ void BytecodeFlowGraphBuilder::BuildAllocateContext() {
965965
const intptr_t context_id = DecodeOperandA().value();
966966
const intptr_t num_context_vars = DecodeOperandE().value();
967967

968-
auto& context_variables = CompilerState::Current().GetDummyContextVariables(
968+
auto& context_slots = CompilerState::Current().GetDummyContextSlots(
969969
context_id, num_context_vars);
970-
code_ += B->AllocateContext(context_variables);
970+
code_ += B->AllocateContext(context_slots);
971971
}
972972

973973
void BytecodeFlowGraphBuilder::BuildCloneContext() {
@@ -979,10 +979,10 @@ void BytecodeFlowGraphBuilder::BuildCloneContext() {
979979
const intptr_t context_id = DecodeOperandA().value();
980980
const intptr_t num_context_vars = DecodeOperandE().value();
981981

982-
auto& context_variables = CompilerState::Current().GetDummyContextVariables(
982+
auto& context_slots = CompilerState::Current().GetDummyContextSlots(
983983
context_id, num_context_vars);
984984
CloneContextInstr* clone_instruction = new (Z) CloneContextInstr(
985-
TokenPosition::kNoSource, Pop(), context_variables, B->GetNextDeoptId());
985+
TokenPosition::kNoSource, Pop(), context_slots, B->GetNextDeoptId());
986986
code_ <<= clone_instruction;
987987
B->Push(clone_instruction);
988988
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1633,8 +1633,8 @@ Fragment StreamingFlowGraphBuilder::AllocateObject(TokenPosition position,
16331633
}
16341634

16351635
Fragment StreamingFlowGraphBuilder::AllocateContext(
1636-
const GrowableArray<LocalVariable*>& context_variables) {
1637-
return flow_graph_builder_->AllocateContext(context_variables);
1636+
const ZoneGrowableArray<const Slot*>& context_slots) {
1637+
return flow_graph_builder_->AllocateContext(context_slots);
16381638
}
16391639

16401640
Fragment StreamingFlowGraphBuilder::LoadNativeField(const Slot& field) {
@@ -1691,8 +1691,8 @@ Fragment StreamingFlowGraphBuilder::CheckStackOverflow(TokenPosition position) {
16911691
}
16921692

16931693
Fragment StreamingFlowGraphBuilder::CloneContext(
1694-
const GrowableArray<LocalVariable*>& context_variables) {
1695-
return flow_graph_builder_->CloneContext(context_variables);
1694+
const ZoneGrowableArray<const Slot*>& context_slots) {
1695+
return flow_graph_builder_->CloneContext(context_slots);
16961696
}
16971697

16981698
Fragment StreamingFlowGraphBuilder::TranslateFinallyFinalizers(
@@ -3049,7 +3049,7 @@ Fragment StreamingFlowGraphBuilder::BuildStaticInvocation(bool is_const,
30493049
++argument_count;
30503050
}
30513051

3052-
if (compiler::ffi::IsAsFunctionInternal(Z, Isolate::Current(), target)) {
3052+
if (compiler::ffi::IsAsFunctionInternal(Z, H.isolate(), target)) {
30533053
return BuildFfiAsFunctionInternal();
30543054
}
30553055

@@ -4137,7 +4137,7 @@ Fragment StreamingFlowGraphBuilder::BuildForStatement() {
41374137
// the body gets a fresh set of [ForStatement] variables (with the old
41384138
// (possibly updated) values).
41394139
if (context_scope->num_context_variables() > 0) {
4140-
body += CloneContext(context_scope->context_variables());
4140+
body += CloneContext(context_scope->context_slots());
41414141
}
41424142

41434143
body += updates;

‎runtime/vm/compiler/frontend/kernel_binary_flowgraph.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,7 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
210210
const Class& klass,
211211
intptr_t argument_count);
212212
Fragment AllocateObject(const Class& klass, const Function& closure_function);
213-
Fragment AllocateContext(
214-
const GrowableArray<LocalVariable*>& context_variables);
213+
Fragment AllocateContext(const ZoneGrowableArray<const Slot*>& context_slots);
215214
Fragment LoadNativeField(const Slot& field);
216215
Fragment StoreLocal(TokenPosition position, LocalVariable* variable);
217216
Fragment StoreStaticField(TokenPosition position, const Field& field);
@@ -224,7 +223,7 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
224223
Fragment CreateArray();
225224
Fragment StoreIndexed(intptr_t class_id);
226225
Fragment CheckStackOverflow(TokenPosition position);
227-
Fragment CloneContext(const GrowableArray<LocalVariable*>& context_variables);
226+
Fragment CloneContext(const ZoneGrowableArray<const Slot*>& context_slots);
228227
Fragment TranslateFinallyFinalizers(TryFinallyBlock* outer_finally,
229228
intptr_t target_context_depth);
230229
Fragment BranchIfTrue(TargetEntryInstr** then_entry,

0 commit comments

Comments
 (0)