X86: Replace VarHandle.get() return type check with CheckCast node
This commit removes the varType check against the callsite return type.
The check is done after by an added HCheckCast node.
Test: art/test.py --host --32 -r -t 712-varhandle-invocations
Bug: 65872996
Change-Id: If4d966e0087da28349390474188e10dfb6f63832
diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index 2da19ba..f5cdf36 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -1148,10 +1148,6 @@
DCHECK_EQ(1 + ArtMethod::NumArgRegisters(shorty), operands.GetNumberOfOperands());
DataType::Type return_type = DataType::FromShorty(shorty[0]);
size_t number_of_arguments = strlen(shorty);
- // Other inputs are needed to propagate type information regarding the MethodType of the
- // call site. We need this when generating code to check that VarHandle accessors are
- // called correctly (for references).
- size_t number_of_other_inputs = 0u;
// We use ResolveMethod which is also used in BuildInvoke in order to
// not duplicate code. As such, we need to provide is_string_constructor
// even if we don't need it afterwards.
@@ -1164,36 +1160,34 @@
/* target_method= */ nullptr,
&is_string_constructor);
- bool needs_other_inputs =
- resolved_method->GetIntrinsic() == static_cast<uint32_t>(Intrinsics::kVarHandleGet) &&
- return_type == DataType::Type::kReference &&
- number_of_arguments == 1u;
- if (needs_other_inputs) {
- // The extra argument here is the loaded callsite return type, which needs to be checked
- // against the runtime VarHandle type.
- number_of_other_inputs++;
- }
-
HInvoke* invoke = new (allocator_) HInvokePolymorphic(allocator_,
number_of_arguments,
- number_of_other_inputs,
return_type,
dex_pc,
method_idx,
resolved_method);
- if (needs_other_inputs) {
+ if (!HandleInvoke(invoke, operands, shorty, /* is_unresolved= */ false)) {
+ return false;
+ }
+
+ bool needs_ret_type_check =
+ resolved_method->GetIntrinsic() == static_cast<uint32_t>(Intrinsics::kVarHandleGet) &&
+ return_type == DataType::Type::kReference &&
+ // VarHandle.get() is only implemented for static fields for now.
+ number_of_arguments == 1u;
+ if (needs_ret_type_check) {
ScopedObjectAccess soa(Thread::Current());
ArtMethod* referrer = graph_->GetArtMethod();
dex::TypeIndex ret_type_index = referrer->GetDexFile()->GetProtoId(proto_idx).return_type_idx_;
- HLoadClass* load_cls = BuildLoadClass(ret_type_index, dex_pc);
- size_t last_index = invoke->InputCount() - 1;
- DCHECK(invoke->InputAt(last_index) == nullptr);
- invoke->SetRawInputAt(last_index, load_cls);
+ // Type check is needed because intrinsic implementations do not type check the retrieved
+ // reference.
+ BuildTypeCheck(/* is_instance_of= */ false, invoke, ret_type_index, dex_pc);
+ latest_result_ = current_block_->GetLastInstruction();
}
- return HandleInvoke(invoke, operands, shorty, /* is_unresolved= */ false);
+ return true;
}
@@ -2441,13 +2435,10 @@
AppendInstruction(load_method_type);
}
-void HInstructionBuilder::BuildTypeCheck(const Instruction& instruction,
- uint8_t destination,
- uint8_t reference,
+void HInstructionBuilder::BuildTypeCheck(bool is_instance_of,
+ HInstruction* object,
dex::TypeIndex type_index,
uint32_t dex_pc) {
- HInstruction* object = LoadLocal(reference, DataType::Type::kReference);
-
ScopedObjectAccess soa(Thread::Current());
const DexFile& dex_file = *dex_compilation_unit_->GetDexFile();
Handle<mirror::Class> klass = ResolveClass(soa, type_index);
@@ -2473,7 +2464,7 @@
}
DCHECK(class_or_null != nullptr);
- if (instruction.Opcode() == Instruction::INSTANCE_OF) {
+ if (is_instance_of) {
AppendInstruction(new (allocator_) HInstanceOf(object,
class_or_null,
check_kind,
@@ -2482,9 +2473,7 @@
allocator_,
bitstring_path_to_root,
bitstring_mask));
- UpdateLocal(destination, current_block_->GetLastInstruction());
} else {
- DCHECK_EQ(instruction.Opcode(), Instruction::CHECK_CAST);
// We emit a CheckCast followed by a BoundType. CheckCast is a statement
// which may throw. If it succeeds BoundType sets the new type of `object`
// for all subsequent uses.
@@ -2498,6 +2487,23 @@
bitstring_path_to_root,
bitstring_mask));
AppendInstruction(new (allocator_) HBoundType(object, dex_pc));
+ }
+}
+
+void HInstructionBuilder::BuildTypeCheck(const Instruction& instruction,
+ uint8_t destination,
+ uint8_t reference,
+ dex::TypeIndex type_index,
+ uint32_t dex_pc) {
+ HInstruction* object = LoadLocal(reference, DataType::Type::kReference);
+ bool is_instance_of = instruction.Opcode() == Instruction::INSTANCE_OF;
+
+ BuildTypeCheck(is_instance_of, object, type_index, dex_pc);
+
+ if (is_instance_of) {
+ UpdateLocal(destination, current_block_->GetLastInstruction());
+ } else {
+ DCHECK_EQ(instruction.Opcode(), Instruction::CHECK_CAST);
UpdateLocal(reference, current_block_->GetLastInstruction());
}
}
diff --git a/compiler/optimizing/instruction_builder.h b/compiler/optimizing/instruction_builder.h
index 04f2a22..e75fa52 100644
--- a/compiler/optimizing/instruction_builder.h
+++ b/compiler/optimizing/instruction_builder.h
@@ -203,6 +203,10 @@
uint32_t dex_pc);
// Builds a `HInstanceOf`, or a `HCheckCast` instruction.
+ void BuildTypeCheck(bool is_instance_of,
+ HInstruction* object,
+ dex::TypeIndex type_index,
+ uint32_t dex_pc);
void BuildTypeCheck(const Instruction& instruction,
uint8_t destination,
uint8_t reference,
diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc
index 1c82639..cf064ec 100644
--- a/compiler/optimizing/intrinsics_x86.cc
+++ b/compiler/optimizing/intrinsics_x86.cc
@@ -3093,12 +3093,7 @@
locations->AddTemp(Location::RequiresRegister());
FALLTHROUGH_INTENDED;
case DataType::Type::kInt32:
- locations->SetOut(Location::RequiresRegister());
- break;
case DataType::Type::kReference:
- // The second input is not an instruction argument. It is the callsite return type
- // used to check the compatibility with VarHandle type.
- locations->SetInAt(1, Location::RequiresRegister());
locations->SetOut(Location::RequiresRegister());
break;
default:
@@ -3124,10 +3119,11 @@
mirror::VarHandle::AccessMode access_mode =
mirror::VarHandle::GetAccessModeByIntrinsic(invoke->GetIntrinsic());
const uint32_t access_mode_bit = 1u << static_cast<uint32_t>(access_mode);
- const uint32_t var_type_offset = mirror::VarHandle::VarTypeOffset().Uint32Value();
const uint32_t coordtype0_offset = mirror::VarHandle::CoordinateType0Offset().Uint32Value();
- const uint32_t super_class_offset = mirror::Class::SuperClassOffset().Uint32Value();
+ const uint32_t var_type_offset = mirror::VarHandle::VarTypeOffset().Uint32Value();
+ const uint32_t primitive_type_offset = mirror::Class::PrimitiveTypeOffset().Uint32Value();
DataType::Type type = invoke->GetType();
+ const uint32_t primitive_type = static_cast<uint32_t>(DataTypeToPrimitive(type));
DCHECK_NE(type, DataType::Type::kVoid);
Register temp = locations->GetTemp(0).AsRegister<Register>();
InstructionCodeGeneratorX86* instr_codegen =
@@ -3143,38 +3139,15 @@
// Do not emit read barrier (or unpoison the reference) for comparing to null.
__ cmpl(Address(varhandle_object, coordtype0_offset), Immediate(0));
__ j(kNotEqual, slow_path->GetEntryLabel());
- // For primitive types, we do not need a read barrier when loading a reference only for loading
- // constant field through the reference. For reference types, we deliberately avoid the read
- // barrier, letting the slow path handle the false negatives.
+
+ // Check the varType.primitiveType against the type we're trying to retrieve. Reference types
+ // are also checked later by a HCheckCast node as an additional check.
+ // We do not need a read barrier when loading a reference only for loading a constant field
+ // through the reference.
__ movl(temp, Address(varhandle_object, var_type_offset));
__ MaybeUnpoisonHeapReference(temp);
- // Check the varType against the type we're trying to retrieve.
- if (type == DataType::Type::kReference) {
- // For reference types, check the type's class reference and if it's not an exact match,
- // check if it is an inherited type.
- Register callsite_ret_type = locations->InAt(1).AsRegister<Register>();
- NearLabel check_ret_type_compatibility, ret_type_matched;
-
- __ Bind(&check_ret_type_compatibility);
- __ cmpl(temp, callsite_ret_type);
- __ j(kEqual, &ret_type_matched);
- // Load the super class.
- __ movl(temp, Address(temp, super_class_offset));
- __ MaybeUnpoisonHeapReference(temp);
- // If the super class is null, we reached the root of the hierarchy. The types are not
- // compatible.
- __ cmpl(temp, Immediate(0));
- __ j(kEqual, slow_path->GetEntryLabel());
- __ jmp(&check_ret_type_compatibility);
- __ Bind(&ret_type_matched);
- } else {
- // For primitive types, check the varType.primitiveType field.
- uint32_t primitive_type = static_cast<uint32_t>(DataTypeToPrimitive(type));
- const uint32_t primitive_type_offset = mirror::Class::PrimitiveTypeOffset().Uint32Value();
-
- __ cmpw(Address(temp, primitive_type_offset), Immediate(primitive_type));
- __ j(kNotEqual, slow_path->GetEntryLabel());
- }
+ __ cmpw(Address(temp, primitive_type_offset), Immediate(primitive_type));
+ __ j(kNotEqual, slow_path->GetEntryLabel());
Location out = locations->Out();
// Use 'out' as a temporary register if it's a core register
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 9b984d5..3ad987e 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -4514,7 +4514,6 @@
public:
HInvokePolymorphic(ArenaAllocator* allocator,
uint32_t number_of_arguments,
- uint32_t number_of_other_inputs,
DataType::Type return_type,
uint32_t dex_pc,
uint32_t dex_method_index,
@@ -4525,7 +4524,7 @@
: HInvoke(kInvokePolymorphic,
allocator,
number_of_arguments,
- number_of_other_inputs,
+ /* number_of_other_inputs= */ 0u,
return_type,
dex_pc,
dex_method_index,
diff --git a/compiler/optimizing/reference_type_propagation.cc b/compiler/optimizing/reference_type_propagation.cc
index e4e8ba3..fd7412c 100644
--- a/compiler/optimizing/reference_type_propagation.cc
+++ b/compiler/optimizing/reference_type_propagation.cc
@@ -882,6 +882,8 @@
}
ScopedObjectAccess soa(Thread::Current());
+ // FIXME: Treat InvokePolymorphic separately, as we can get a more specific return type from
+ // protoId than the one obtained from the resolved method.
ArtMethod* method = instr->GetResolvedMethod();
ObjPtr<mirror::Class> klass = (method == nullptr) ? nullptr : method->LookupResolvedReturnType();
SetClassAsTypeInfo(instr, klass, /* is_exact= */ false);