Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 30abce9

Browse files
alexmarkovcommit-bot@chromium.org
authored andcommitted
[vm] Treat static final fields with trivial initializers as const
When loading a value of a static final field with trivial initializer generate a Constant instruction instead of LoadStaticField. Initializer of a static field is considered trivial if it is null, int, double, String or bool literal. TEST=vm/cc/StreamingFlowGraphBuilder_StaticGetFinalFieldWithTrivialInitializer Fixes dart-lang/sdk#47120 Change-Id: Id2bfc3da8c14376f7f1ef829265cca29a018bad1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212873 Reviewed-by: Slava Egorov <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
1 parent 1d6f4c7 commit 30abce9

File tree

5 files changed

+53
-3
lines changed

5 files changed

+53
-3
lines changed

runtime/vm/compiler/backend/type_propagator_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,7 @@ ISOLATE_UNIT_TEST_CASE(TypePropagator_NonNullableLoadStaticField) {
552552

553553
const char* kScript = R"(
554554
const y = 0xDEADBEEF;
555-
final int x = 0xFEEDFEED;
555+
final int x = int.parse('0xFEEDFEED');
556556
557557
void main(List<String> args) {
558558
print(x);

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2776,6 +2776,10 @@ Fragment StreamingFlowGraphBuilder::BuildStaticGet(TokenPosition* p) {
27762776
Class::Handle(field.Owner()).Name() == Symbols::ClassID().ptr());
27772777
return Constant(Instance::ZoneHandle(
27782778
Z, Instance::RawCast(field.StaticConstFieldValue())));
2779+
} else if (field.is_final() && field.has_trivial_initializer()) {
2780+
// Final fields with trivial initializers are effectively constant.
2781+
return Constant(Instance::ZoneHandle(
2782+
Z, Instance::RawCast(field.StaticConstFieldValue())));
27792783
} else {
27802784
const Class& owner = Class::Handle(Z, field.Owner());
27812785
const String& getter_name = H.DartGetterName(target);

runtime/vm/compiler/frontend/kernel_binary_flowgraph_test.cc

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,4 +340,44 @@ ISOLATE_UNIT_TEST_CASE(StreamingFlowGraphBuilder_TypedClosureCall) {
340340
// clang-format on
341341
}
342342

343+
ISOLATE_UNIT_TEST_CASE(
344+
StreamingFlowGraphBuilder_StaticGetFinalFieldWithTrivialInitializer) {
345+
const char* kScript = R"(
346+
final int x = 0xFEEDFEED;
347+
test() {
348+
return x;
349+
}
350+
)";
351+
352+
const auto& root_library = Library::Handle(LoadTestScript(kScript));
353+
const auto& function = Function::Handle(GetFunction(root_library, "test"));
354+
355+
Invoke(root_library, "test");
356+
357+
TestPipeline pipeline(function, CompilerPass::kJIT);
358+
FlowGraph* flow_graph = pipeline.RunPasses({
359+
CompilerPass::kComputeSSA,
360+
});
361+
362+
auto entry = flow_graph->graph_entry()->normal_entry();
363+
EXPECT(entry != nullptr);
364+
365+
ReturnInstr* return_instr = nullptr;
366+
367+
ILMatcher cursor(flow_graph, entry);
368+
RELEASE_ASSERT(cursor.TryMatch({
369+
kMatchAndMoveFunctionEntry,
370+
kMatchAndMoveCheckStackOverflow,
371+
kMoveDebugStepChecks,
372+
{kMatchReturn, &return_instr},
373+
}));
374+
375+
EXPECT(return_instr != nullptr);
376+
ConstantInstr* const_value =
377+
return_instr->value()->definition()->AsConstant();
378+
EXPECT(const_value != nullptr);
379+
EXPECT(const_value->value().IsInteger());
380+
EXPECT_EQ(0xFEEDFEED, Integer::Cast(const_value->value()).AsInt64Value());
381+
}
382+
343383
} // namespace dart

runtime/vm/kernel_loader.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -881,7 +881,8 @@ void KernelLoader::CheckForInitializer(const Field& field) {
881881
SimpleExpressionConverter converter(&H, &helper_);
882882
const bool has_simple_initializer =
883883
converter.IsSimple(helper_.ReaderOffset() + 1);
884-
if (!has_simple_initializer || !converter.SimpleValue().IsNull()) {
884+
if (!has_simple_initializer ||
885+
(!field.is_static() && !converter.SimpleValue().IsNull())) {
885886
field.set_has_nontrivial_initializer(true);
886887
}
887888
return;

runtime/vm/object.cc

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11172,7 +11172,8 @@ ErrorPtr Field::InitializeStatic() const {
1117211172
}
1117311173

1117411174
ObjectPtr Field::StaticConstFieldValue() const {
11175-
ASSERT(is_static() && is_const());
11175+
ASSERT(is_static() &&
11176+
(is_const() || (is_final() && has_trivial_initializer())));
1117611177

1117711178
auto thread = Thread::Current();
1117811179
auto zone = thread->zone();
@@ -11183,7 +11184,11 @@ ObjectPtr Field::StaticConstFieldValue() const {
1118311184
auto& value = Object::Handle(
1118411185
zone, initial_field_table->At(field_id(), /*concurrent_use=*/true));
1118511186
if (value.ptr() == Object::sentinel().ptr()) {
11187+
// Fields with trivial initializers get their initial value
11188+
// eagerly when they are registered.
11189+
ASSERT(is_const());
1118611190
ASSERT(has_initializer());
11191+
ASSERT(has_nontrivial_initializer());
1118711192
value = EvaluateInitializer();
1118811193
if (!value.IsError()) {
1118911194
ASSERT(value.IsNull() || value.IsInstance());

0 commit comments

Comments
 (0)