Revert^2 "X86: VarHandle.get() for reference type static fields."
This reverts commit 6a6cca588df43a180534db0b49475a8a5ab4c35a.
This commit extends the VarHandle.get() implementation to work with
reference type fields, not only primitive types.
Test: ART_HEAP_POISONING=true art/test.py --host --32 -r -t 712-varhandle-invocations
Bug: 65872996
Reason for revert: Fix loading the reference field.
Change-Id: Ide1f13fb9c6a4e97876862fb4772e9d543847f1f
diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index 6839292..1993fa2 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -1148,6 +1148,10 @@
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.
@@ -1159,12 +1163,36 @@
&invoke_type,
/* 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) {
+ 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);
+ }
+
return HandleInvoke(invoke, operands, shorty, /* is_unresolved= */ false);
}
diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc
index 2fdea76..1c82639 100644
--- a/compiler/optimizing/intrinsics_x86.cc
+++ b/compiler/optimizing/intrinsics_x86.cc
@@ -3080,12 +3080,6 @@
return;
}
- if (type == DataType::Type::kReference) {
- // Reference return type is not implemented yet
- // TODO: implement for kReference
- return;
- }
-
if (invoke->GetNumberOfArguments() == 1u) {
// Static field get
ArenaAllocator* allocator = invoke->GetBlock()->GetGraph()->GetAllocator();
@@ -3101,6 +3095,12 @@
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:
DCHECK(DataType::IsFloatingPointType(type));
locations->AddTemp(Location::RequiresRegister());
@@ -3126,13 +3126,12 @@
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 primitive_type_offset = mirror::Class::PrimitiveTypeOffset().Uint32Value();
+ const uint32_t super_class_offset = mirror::Class::SuperClassOffset().Uint32Value();
DataType::Type type = invoke->GetType();
- // For now, only primitive types are supported
DCHECK_NE(type, DataType::Type::kVoid);
- DCHECK_NE(type, DataType::Type::kReference);
- uint32_t primitive_type = static_cast<uint32_t>(DataTypeToPrimitive(type));
Register temp = locations->GetTemp(0).AsRegister<Register>();
+ InstructionCodeGeneratorX86* instr_codegen =
+ down_cast<InstructionCodeGeneratorX86*>(codegen_->GetInstructionVisitor());
// If the access mode is not supported, bail to runtime implementation to handle
__ testl(Address(varhandle_object, access_modes_bitmask_offset), Immediate(access_mode_bit));
@@ -3140,17 +3139,42 @@
codegen_->AddSlowPath(slow_path);
__ j(kZero, slow_path->GetEntryLabel());
- // Check the varType.primitiveType against the type we're trying to retrieve. We do not need a
- // read barrier when loading a reference only for loading constant field through the reference.
- __ movl(temp, Address(varhandle_object, var_type_offset));
- __ MaybeUnpoisonHeapReference(temp);
- __ cmpw(Address(temp, primitive_type_offset), Immediate(primitive_type));
- __ j(kNotEqual, slow_path->GetEntryLabel());
-
// Check that the varhandle references a static field by checking that coordinateType0 == null.
// 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.
+ __ 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());
+ }
Location out = locations->Out();
// Use 'out' as a temporary register if it's a core register
@@ -3163,8 +3187,6 @@
// Load the ArtField, the offset and declaring class
__ movl(temp, Address(varhandle_object, artfield_offset));
__ movl(offset, Address(temp, offset_offset));
- InstructionCodeGeneratorX86* instr_codegen =
- down_cast<InstructionCodeGeneratorX86*>(codegen_->GetInstructionVisitor());
instr_codegen->GenerateGcRootFieldLoad(invoke,
Location::RegisterLocation(temp),
Address(temp, declaring_class_offset),
@@ -3173,7 +3195,20 @@
// Load the value from the field
CodeGeneratorX86* codegen_x86 = down_cast<CodeGeneratorX86*>(codegen_);
- codegen_x86->MoveFromMemory(type, out, temp, offset);
+ if (type == DataType::Type::kReference) {
+ if (kCompilerReadBarrierOption == kWithReadBarrier) {
+ codegen_x86->GenerateReferenceLoadWithBakerReadBarrier(invoke,
+ out,
+ temp,
+ Address(temp, offset, TIMES_1, 0),
+ /* needs_null_check= */ false);
+ } else {
+ __ movl(out.AsRegister<Register>(), Address(temp, offset, TIMES_1, 0));
+ __ MaybeUnpoisonHeapReference(out.AsRegister<Register>());
+ }
+ } else {
+ codegen_x86->MoveFromMemory(type, out, temp, offset);
+ }
__ Bind(slow_path->GetExitLabel());
}
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 3ad987e..9b984d5 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -4514,6 +4514,7 @@
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,
@@ -4524,7 +4525,7 @@
: HInvoke(kInvokePolymorphic,
allocator,
number_of_arguments,
- /* number_of_other_inputs= */ 0u,
+ number_of_other_inputs,
return_type,
dex_pc,
dex_method_index,