From 8159c38d6533eb525ba4b86f87db91a36c571b64 Mon Sep 17 00:00:00 2001 From: Alexander Aprelev Date: Tue, 30 Nov 2021 08:10:25 +0000 Subject: [PATCH] [vm/jit/unboxed] Remove heuristic that transitions unboxed fields back to boxed. The heuristic was introduced in b870dafa4f04a117366292b5824739d8f1806fb0, but gets in the way of maintaining Slot(extended with field guard information) immutable during compilation. Existing set of benchmarks doesn't seem to show any reasonable regression with this heuristic removed. Bug: https://github.com/dart-lang/sdk/issues/47722 Change-Id: Ie94050d96852b7ffef2dc81248cda23e84d41f4b TEST=ci Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/221544 Reviewed-by: Slava Egorov Reviewed-by: Ryan Macnak Commit-Queue: Alexander Aprelev --- .../vm/compiler/jit/jit_call_specializer.cc | 51 ------------------- .../vm/compiler/jit/jit_call_specializer.h | 1 - runtime/vm/flag_list.h | 2 - 3 files changed, 54 deletions(-) diff --git a/runtime/vm/compiler/jit/jit_call_specializer.cc b/runtime/vm/compiler/jit/jit_call_specializer.cc index 191e6bb3e9e..3afc611fe43 100644 --- a/runtime/vm/compiler/jit/jit_call_specializer.cc +++ b/runtime/vm/compiler/jit/jit_call_specializer.cc @@ -162,57 +162,6 @@ void JitCallSpecializer::VisitInstanceCall(InstanceCallInstr* instr) { } } -void JitCallSpecializer::VisitStoreInstanceField( - StoreInstanceFieldInstr* instr) { - if (instr->IsUnboxedDartFieldStore()) { - // Determine if this field should be unboxed based on the usage of getter - // and setter functions: The heuristic requires that the setter has a - // usage count of at least 1/kGetterSetterRatio of the getter usage count. - // This is to avoid unboxing fields where the setter is never or rarely - // executed. - const Field& field = instr->slot().field(); - const String& field_name = String::Handle(Z, field.name()); - const Class& owner = Class::Handle(Z, field.Owner()); - const Function& getter = - Function::Handle(Z, owner.LookupGetterFunction(field_name)); - const Function& setter = - Function::Handle(Z, owner.LookupSetterFunction(field_name)); - bool unboxed_field = false; - if (!getter.IsNull() && !setter.IsNull()) { - if (field.is_double_initialized()) { - unboxed_field = true; - } else if ((setter.usage_counter() > 0) && - ((FLAG_getter_setter_ratio * setter.usage_counter()) >= - getter.usage_counter())) { - unboxed_field = true; - } - } - if (!unboxed_field) { - if (FLAG_trace_optimization || FLAG_trace_field_guards) { - THR_Print("Disabling unboxing of %s\n", field.ToCString()); - if (!setter.IsNull()) { - THR_Print(" setter usage count: %" Pd "\n", setter.usage_counter()); - } - if (!getter.IsNull()) { - THR_Print(" getter usage count: %" Pd "\n", getter.usage_counter()); - } - } - // We determined it's not beneficial for performance to unbox the - // field, therefore we mark it as boxed here. - // - // Calling `DisableFieldUnboxing` will cause transition the field to - // boxed and deoptimize dependent code. - // - // NOTE: It will also, as a side-effect, change our field clone's - // `is_unboxing_candidate()` bit. So we assume the compiler has so far - // not relied on this bit. - field.DisableFieldUnboxing(); - } else { - flow_graph()->parsed_function().AddToGuardedFields(&field); - } - } -} - // Replace generic context allocation or cloning with a sequence of inlined // allocation and explicit initializing stores. // If context_value is not NULL then newly allocated context is a populated diff --git a/runtime/vm/compiler/jit/jit_call_specializer.h b/runtime/vm/compiler/jit/jit_call_specializer.h index 8c743ed26e0..ecbee73b18b 100644 --- a/runtime/vm/compiler/jit/jit_call_specializer.h +++ b/runtime/vm/compiler/jit/jit_call_specializer.h @@ -27,7 +27,6 @@ class JitCallSpecializer : public CallSpecializer { // Find a better place for them. virtual void VisitAllocateContext(AllocateContextInstr* instr); virtual void VisitCloneContext(CloneContextInstr* instr); - virtual void VisitStoreInstanceField(StoreInstanceFieldInstr* instr); private: virtual bool IsAllowedForInlining(intptr_t deopt_id) const; diff --git a/runtime/vm/flag_list.h b/runtime/vm/flag_list.h index 679bf26b192..edf42f7e577 100644 --- a/runtime/vm/flag_list.h +++ b/runtime/vm/flag_list.h @@ -125,8 +125,6 @@ constexpr bool FLAG_support_il_printer = false; P(enable_ffi, bool, true, "Disable to make importing dart:ffi an error.") \ P(force_clone_compiler_objects, bool, false, \ "Force cloning of objects needed in compiler (ICData and Field).") \ - P(getter_setter_ratio, int, 13, \ - "Ratio of getter/setter usage used for double field unboxing heuristics") \ P(guess_icdata_cid, bool, true, \ "Artificially create type feedback for arithmetic etc. operations") \ P(huge_method_cutoff_in_tokens, int, 20000, \