ART: Run RTP if invoke inputs have more specific type
Moving RTP inside the graph builder introduced a regression where
replacing the inner parameters with the actual arguments of the
HInvoke would not build the inner graph with types more specific
than the method's signature.
This patch runs RTP on the inner graph again when it is detected
that RTP may improve typing precision.
Bug: 29595335
Change-Id: I351babc8497c83c2fba589aa51f46eaa0b7ab33c
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index d5e80b4..be4ea20 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -756,7 +756,15 @@
invoke_instruction->ReplaceWith(return_replacement);
}
invoke_instruction->GetBlock()->RemoveInstruction(invoke_instruction);
- FixUpReturnReferenceType(invoke_instruction, method, return_replacement, do_rtp);
+ FixUpReturnReferenceType(method, return_replacement);
+ if (do_rtp && ReturnTypeMoreSpecific(invoke_instruction, return_replacement)) {
+ // Actual return value has a more specific type than the method's declared
+ // return type. Run RTP again on the outer graph to propagate it.
+ ReferenceTypePropagation(graph_,
+ outer_compilation_unit_.GetDexCache(),
+ handles_,
+ /* is_first_run */ false).Run();
+ }
return true;
}
@@ -1159,6 +1167,15 @@
}
}
+ // We have replaced formal arguments with actual arguments. If actual types
+ // are more specific than the declared ones, run RTP again on the inner graph.
+ if (ArgumentTypesMoreSpecific(invoke_instruction, resolved_method)) {
+ ReferenceTypePropagation(callee_graph,
+ dex_compilation_unit.GetDexCache(),
+ handles_,
+ /* is_first_run */ false).Run();
+ }
+
size_t number_of_instructions_budget = kMaximumNumberOfHInstructions;
size_t number_of_inlined_instructions =
RunOptimizations(callee_graph, code_item, dex_compilation_unit);
@@ -1332,13 +1349,87 @@
return number_of_inlined_instructions;
}
-void HInliner::FixUpReturnReferenceType(HInvoke* invoke_instruction,
- ArtMethod* resolved_method,
- HInstruction* return_replacement,
- bool do_rtp) {
+static bool IsReferenceTypeRefinement(ReferenceTypeInfo declared_rti,
+ bool declared_can_be_null,
+ HInstruction* actual_obj)
+ SHARED_REQUIRES(Locks::mutator_lock_) {
+ if (declared_can_be_null && !actual_obj->CanBeNull()) {
+ return true;
+ }
+
+ ReferenceTypeInfo actual_rti = actual_obj->GetReferenceTypeInfo();
+ return (actual_rti.IsExact() && !declared_rti.IsExact()) ||
+ declared_rti.IsStrictSupertypeOf(actual_rti);
+}
+
+ReferenceTypeInfo HInliner::GetClassRTI(mirror::Class* klass) {
+ return ReferenceTypePropagation::IsAdmissible(klass)
+ ? ReferenceTypeInfo::Create(handles_->NewHandle(klass))
+ : graph_->GetInexactObjectRti();
+}
+
+bool HInliner::ArgumentTypesMoreSpecific(HInvoke* invoke_instruction, ArtMethod* resolved_method) {
+ // If this is an instance call, test whether the type of the `this` argument
+ // is more specific than the class which declares the method.
+ if (!resolved_method->IsStatic()) {
+ if (IsReferenceTypeRefinement(GetClassRTI(resolved_method->GetDeclaringClass()),
+ /* declared_can_be_null */ false,
+ invoke_instruction->InputAt(0u))) {
+ return true;
+ }
+ }
+
+ size_t pointer_size = Runtime::Current()->GetClassLinker()->GetImagePointerSize();
+
+ // Iterate over the list of parameter types and test whether any of the
+ // actual inputs has a more specific reference type than the type declared in
+ // the signature.
+ const DexFile::TypeList* param_list = resolved_method->GetParameterTypeList();
+ for (size_t param_idx = 0,
+ input_idx = resolved_method->IsStatic() ? 0 : 1,
+ e = (param_list == nullptr ? 0 : param_list->Size());
+ param_idx < e;
+ ++param_idx, ++input_idx) {
+ HInstruction* input = invoke_instruction->InputAt(input_idx);
+ if (input->GetType() == Primitive::kPrimNot) {
+ mirror::Class* param_cls = resolved_method->GetDexCacheResolvedType(
+ param_list->GetTypeItem(param_idx).type_idx_,
+ pointer_size);
+ if (IsReferenceTypeRefinement(GetClassRTI(param_cls),
+ /* declared_can_be_null */ true,
+ input)) {
+ return true;
+ }
+ }
+ }
+
+ return false;
+}
+
+bool HInliner::ReturnTypeMoreSpecific(HInvoke* invoke_instruction,
+ HInstruction* return_replacement) {
// Check the integrity of reference types and run another type propagation if needed.
if (return_replacement != nullptr) {
if (return_replacement->GetType() == Primitive::kPrimNot) {
+ // Test if the return type is a refinement of the declared return type.
+ if (IsReferenceTypeRefinement(invoke_instruction->GetReferenceTypeInfo(),
+ /* declared_can_be_null */ true,
+ return_replacement)) {
+ return true;
+ }
+ } else if (return_replacement->IsInstanceOf()) {
+ // Inlining InstanceOf into an If may put a tighter bound on reference types.
+ return true;
+ }
+ }
+
+ return false;
+}
+
+void HInliner::FixUpReturnReferenceType(ArtMethod* resolved_method,
+ HInstruction* return_replacement) {
+ if (return_replacement != nullptr) {
+ if (return_replacement->GetType() == Primitive::kPrimNot) {
if (!return_replacement->GetReferenceTypeInfo().IsValid()) {
// Make sure that we have a valid type for the return. We may get an invalid one when
// we inline invokes with multiple branches and create a Phi for the result.
@@ -1347,36 +1438,7 @@
DCHECK(return_replacement->IsPhi());
size_t pointer_size = Runtime::Current()->GetClassLinker()->GetImagePointerSize();
mirror::Class* cls = resolved_method->GetReturnType(false /* resolve */, pointer_size);
- if (cls != nullptr && !cls->IsErroneous()) {
- ReferenceTypeInfo::TypeHandle return_handle = handles_->NewHandle(cls);
- return_replacement->SetReferenceTypeInfo(ReferenceTypeInfo::Create(
- return_handle, return_handle->CannotBeAssignedFromOtherTypes() /* is_exact */));
- } else {
- // Return inexact object type on failures.
- return_replacement->SetReferenceTypeInfo(graph_->GetInexactObjectRti());
- }
- }
-
- if (do_rtp) {
- // If the return type is a refinement of the declared type run the type propagation again.
- ReferenceTypeInfo return_rti = return_replacement->GetReferenceTypeInfo();
- ReferenceTypeInfo invoke_rti = invoke_instruction->GetReferenceTypeInfo();
- if (invoke_rti.IsStrictSupertypeOf(return_rti)
- || (return_rti.IsExact() && !invoke_rti.IsExact())
- || !return_replacement->CanBeNull()) {
- ReferenceTypePropagation(graph_,
- outer_compilation_unit_.GetDexCache(),
- handles_,
- /* is_first_run */ false).Run();
- }
- }
- } else if (return_replacement->IsInstanceOf()) {
- if (do_rtp) {
- // Inlining InstanceOf into an If may put a tighter bound on reference types.
- ReferenceTypePropagation(graph_,
- outer_compilation_unit_.GetDexCache(),
- handles_,
- /* is_first_run */ false).Run();
+ return_replacement->SetReferenceTypeInfo(GetClassRTI(cls));
}
}
}
diff --git a/compiler/optimizing/inliner.h b/compiler/optimizing/inliner.h
index 7cf1424..02d3a5f 100644
--- a/compiler/optimizing/inliner.h
+++ b/compiler/optimizing/inliner.h
@@ -124,10 +124,18 @@
uint32_t dex_pc) const
SHARED_REQUIRES(Locks::mutator_lock_);
- void FixUpReturnReferenceType(HInvoke* invoke_instruction,
- ArtMethod* resolved_method,
- HInstruction* return_replacement,
- bool do_rtp)
+ void FixUpReturnReferenceType(ArtMethod* resolved_method, HInstruction* return_replacement)
+ SHARED_REQUIRES(Locks::mutator_lock_);
+
+ // Creates an instance of ReferenceTypeInfo from `klass` if `klass` is
+ // admissible (see ReferenceTypePropagation::IsAdmissible for details).
+ // Otherwise returns inexact Object RTI.
+ ReferenceTypeInfo GetClassRTI(mirror::Class* klass) SHARED_REQUIRES(Locks::mutator_lock_);
+
+ bool ArgumentTypesMoreSpecific(HInvoke* invoke_instruction, ArtMethod* resolved_method)
+ SHARED_REQUIRES(Locks::mutator_lock_);
+
+ bool ReturnTypeMoreSpecific(HInvoke* invoke_instruction, HInstruction* return_replacement)
SHARED_REQUIRES(Locks::mutator_lock_);
// Add a type guard on the given `receiver`. This will add to the graph:
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 711a6c1..18a00d0 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -161,6 +161,10 @@
static ReferenceTypeInfo Create(TypeHandle type_handle, bool is_exact);
+ static ReferenceTypeInfo Create(TypeHandle type_handle) SHARED_REQUIRES(Locks::mutator_lock_) {
+ return Create(type_handle, type_handle->CannotBeAssignedFromOtherTypes());
+ }
+
static ReferenceTypeInfo CreateUnchecked(TypeHandle type_handle, bool is_exact) {
return ReferenceTypeInfo(type_handle, is_exact);
}
diff --git a/compiler/optimizing/reference_type_propagation.cc b/compiler/optimizing/reference_type_propagation.cc
index 3e6adcb..3dfd728 100644
--- a/compiler/optimizing/reference_type_propagation.cc
+++ b/compiler/optimizing/reference_type_propagation.cc
@@ -46,13 +46,6 @@
return *cache;
}
-// Returns true if klass is admissible to the propagation: non-null and resolved.
-// For an array type, we also check if the component type is admissible.
-static bool IsAdmissible(mirror::Class* klass) SHARED_REQUIRES(Locks::mutator_lock_) {
- return klass != nullptr && klass->IsResolved() &&
- (!klass->IsArrayClass() || IsAdmissible(klass->GetComponentType()));
-}
-
ReferenceTypeInfo::TypeHandle ReferenceTypePropagation::HandleCache::GetObjectClassHandle() {
return GetRootHandle(handles_, ClassLinker::kJavaLangObject, &object_class_handle_);
}
diff --git a/compiler/optimizing/reference_type_propagation.h b/compiler/optimizing/reference_type_propagation.h
index 2106be6..edd83bf 100644
--- a/compiler/optimizing/reference_type_propagation.h
+++ b/compiler/optimizing/reference_type_propagation.h
@@ -42,6 +42,14 @@
void Run() OVERRIDE;
+ // Returns true if klass is admissible to the propagation: non-null and resolved.
+ // For an array type, we also check if the component type is admissible.
+ static bool IsAdmissible(mirror::Class* klass) SHARED_REQUIRES(Locks::mutator_lock_) {
+ return klass != nullptr &&
+ klass->IsResolved() &&
+ (!klass->IsArrayClass() || IsAdmissible(klass->GetComponentType()));
+ }
+
static constexpr const char* kReferenceTypePropagationPassName = "reference_type_propagation";
private:
diff --git a/test/450-checker-types/src/Main.java b/test/450-checker-types/src/Main.java
index 08b6cec..98a0938 100644
--- a/test/450-checker-types/src/Main.java
+++ b/test/450-checker-types/src/Main.java
@@ -30,6 +30,11 @@
public void $noinline$f() {
throw new RuntimeException();
}
+
+ public int $inline$h(boolean cond) {
+ Super obj = (cond ? this : null);
+ return obj.hashCode();
+ }
}
class SubclassA extends Super {
@@ -620,6 +625,45 @@
o.mainField = 0;
}
+ /// CHECK-START: void Main.testThisArgumentMoreSpecific(boolean) inliner (before)
+ /// CHECK-DAG: <<Arg:l\d+>> NewInstance
+ /// CHECK-DAG: InvokeVirtual [<<Arg>>,{{z\d+}}] method_name:Super.$inline$h
+
+ /// CHECK-START: void Main.testThisArgumentMoreSpecific(boolean) inliner (after)
+ /// CHECK-DAG: <<Arg:l\d+>> NewInstance
+ /// CHECK-DAG: <<Null:l\d+>> NullConstant
+ /// CHECK-DAG: <<Phi:l\d+>> Phi [<<Arg>>,<<Null>>] klass:SubclassA
+ /// CHECK-DAG: <<NCPhi:l\d+>> NullCheck [<<Phi>>]
+ /// CHECK-DAG: InvokeVirtual [<<NCPhi>>] method_name:Super.hashCode
+
+ public void testThisArgumentMoreSpecific(boolean cond) {
+ // Inlining method from Super will build it with `this` typed as Super.
+ // Running RTP will sharpen it to SubclassA.
+ SubclassA obj = new SubclassA();
+ ((Super) obj).$inline$h(cond);
+ }
+
+ public static int $inline$hashCode(Super obj) {
+ return obj.hashCode();
+ }
+
+ /// CHECK-START: void Main.testExplicitArgumentMoreSpecific(SubclassA) inliner (before)
+ /// CHECK-DAG: <<Arg:l\d+>> ParameterValue klass:SubclassA
+ /// CHECK-DAG: InvokeStaticOrDirect [<<Arg>>] method_name:Main.$inline$hashCode
+
+ /// CHECK-START: void Main.testExplicitArgumentMoreSpecific(SubclassA) inliner (after)
+ /// CHECK-DAG: <<Arg:l\d+>> ParameterValue klass:SubclassA
+ /// CHECK-DAG: <<NCArg:l\d+>> NullCheck [<<Arg>>] klass:SubclassA
+ /// CHECK-DAG: InvokeVirtual [<<NCArg>>] method_name:Super.hashCode
+
+ public void testExplicitArgumentMoreSpecific(SubclassA obj) {
+ // Inlining a method will build it with reference types from its signature,
+ // here the callee graph is built with Super as the type of its only argument.
+ // Running RTP after its ParameterValue instructions are replaced with actual
+ // arguments will type the inner graph more precisely.
+ $inline$hashCode(obj);
+ }
+
/// CHECK-START: void Main.testPhiHasOnlyNullInputs(boolean) inliner (before)
/// CHECK: <<Int:i\d+>> IntConstant 0
/// CHECK: <<Phi:l\d+>> Phi klass:Main exact:false