X86: Add the other set VarHandles (setVolatile, setRelease, setOpaque)
This commit implements VarHandle.setVolatile, setRelease and setOpaque
intrisics. This also implied refactoring HandleFieldSet to be reused in
all set VarHandles, as the code is very similar.
Test: ART_HEAP_POISONING=true art/test.py --host --all-compiler -r --32
Test: ART_HEAP_POISONING=false art/test.py --host --all-compiler -r --32
Test: ART_USE_READ_BARRIER=true art/test.py --host --all-compiler -r --32
Test: ART_USE_READ_BARRIER=false art/test.py --host --all-compiler -r --32
Bug: 65872996
Change-Id: I9a1d5fec6c5086c1e77ba65c3337da1133b3e3f1
diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc
index 1a83976..d3e58ed 100644
--- a/compiler/optimizing/code_generator_x86.cc
+++ b/compiler/optimizing/code_generator_x86.cc
@@ -5722,18 +5722,16 @@
}
void InstructionCodeGeneratorX86::HandleFieldSet(HInstruction* instruction,
- const FieldInfo& field_info,
+ uint32_t value_index,
+ DataType::Type field_type,
+ Address field_addr,
+ Register base,
+ bool is_volatile,
bool value_can_be_null) {
- DCHECK(instruction->IsInstanceFieldSet() || instruction->IsStaticFieldSet());
-
LocationSummary* locations = instruction->GetLocations();
- Register base = locations->InAt(0).AsRegister<Register>();
- Location value = locations->InAt(1);
- bool is_volatile = field_info.IsVolatile();
- DataType::Type field_type = field_info.GetFieldType();
- uint32_t offset = field_info.GetFieldOffset().Uint32Value();
+ Location value = locations->InAt(value_index);
bool needs_write_barrier =
- CodeGenerator::StoreNeedsWriteBarrier(field_type, instruction->InputAt(1));
+ CodeGenerator::StoreNeedsWriteBarrier(field_type, instruction->InputAt(value_index));
if (is_volatile) {
codegen_->GenerateMemoryBarrier(MemBarrierKind::kAnyStore);
@@ -5745,17 +5743,20 @@
case DataType::Type::kBool:
case DataType::Type::kUint8:
case DataType::Type::kInt8: {
- __ movb(Address(base, offset), value.AsRegister<ByteRegister>());
+ if (value.IsConstant()) {
+ __ movb(field_addr, Immediate(CodeGenerator::GetInt8ValueOf(value.GetConstant())));
+ } else {
+ __ movb(field_addr, value.AsRegister<ByteRegister>());
+ }
break;
}
case DataType::Type::kUint16:
case DataType::Type::kInt16: {
if (value.IsConstant()) {
- __ movw(Address(base, offset),
- Immediate(CodeGenerator::GetInt16ValueOf(value.GetConstant())));
+ __ movw(field_addr, Immediate(CodeGenerator::GetInt16ValueOf(value.GetConstant())));
} else {
- __ movw(Address(base, offset), value.AsRegister<Register>());
+ __ movw(field_addr, value.AsRegister<Register>());
}
break;
}
@@ -5770,13 +5771,13 @@
Register temp = locations->GetTemp(0).AsRegister<Register>();
__ movl(temp, value.AsRegister<Register>());
__ PoisonHeapReference(temp);
- __ movl(Address(base, offset), temp);
+ __ movl(field_addr, temp);
} else if (value.IsConstant()) {
int32_t v = CodeGenerator::GetInt32ValueOf(value.GetConstant());
- __ movl(Address(base, offset), Immediate(v));
+ __ movl(field_addr, Immediate(v));
} else {
DCHECK(value.IsRegister()) << value;
- __ movl(Address(base, offset), value.AsRegister<Register>());
+ __ movl(field_addr, value.AsRegister<Register>());
}
break;
}
@@ -5788,17 +5789,17 @@
__ movd(temp1, value.AsRegisterPairLow<Register>());
__ movd(temp2, value.AsRegisterPairHigh<Register>());
__ punpckldq(temp1, temp2);
- __ movsd(Address(base, offset), temp1);
+ __ movsd(field_addr, temp1);
codegen_->MaybeRecordImplicitNullCheck(instruction);
} else if (value.IsConstant()) {
int64_t v = CodeGenerator::GetInt64ValueOf(value.GetConstant());
- __ movl(Address(base, offset), Immediate(Low32Bits(v)));
+ __ movl(field_addr, Immediate(Low32Bits(v)));
codegen_->MaybeRecordImplicitNullCheck(instruction);
- __ movl(Address(base, kX86WordSize + offset), Immediate(High32Bits(v)));
+ __ movl(field_addr.displaceBy(kX86WordSize), Immediate(High32Bits(v)));
} else {
- __ movl(Address(base, offset), value.AsRegisterPairLow<Register>());
+ __ movl(field_addr, value.AsRegisterPairLow<Register>());
codegen_->MaybeRecordImplicitNullCheck(instruction);
- __ movl(Address(base, kX86WordSize + offset), value.AsRegisterPairHigh<Register>());
+ __ movl(field_addr.displaceBy(kX86WordSize), value.AsRegisterPairHigh<Register>());
}
maybe_record_implicit_null_check_done = true;
break;
@@ -5807,9 +5808,9 @@
case DataType::Type::kFloat32: {
if (value.IsConstant()) {
int32_t v = CodeGenerator::GetInt32ValueOf(value.GetConstant());
- __ movl(Address(base, offset), Immediate(v));
+ __ movl(field_addr, Immediate(v));
} else {
- __ movss(Address(base, offset), value.AsFpuRegister<XmmRegister>());
+ __ movss(field_addr, value.AsFpuRegister<XmmRegister>());
}
break;
}
@@ -5818,12 +5819,12 @@
if (value.IsConstant()) {
DCHECK(!is_volatile);
int64_t v = CodeGenerator::GetInt64ValueOf(value.GetConstant());
- __ movl(Address(base, offset), Immediate(Low32Bits(v)));
+ __ movl(field_addr, Immediate(Low32Bits(v)));
codegen_->MaybeRecordImplicitNullCheck(instruction);
- __ movl(Address(base, kX86WordSize + offset), Immediate(High32Bits(v)));
+ __ movl(field_addr.displaceBy(kX86WordSize), Immediate(High32Bits(v)));
maybe_record_implicit_null_check_done = true;
} else {
- __ movsd(Address(base, offset), value.AsFpuRegister<XmmRegister>());
+ __ movsd(field_addr, value.AsFpuRegister<XmmRegister>());
}
break;
}
@@ -5850,6 +5851,28 @@
}
}
+void InstructionCodeGeneratorX86::HandleFieldSet(HInstruction* instruction,
+ const FieldInfo& field_info,
+ bool value_can_be_null) {
+ DCHECK(instruction->IsInstanceFieldSet() || instruction->IsStaticFieldSet());
+
+ LocationSummary* locations = instruction->GetLocations();
+ Register base = locations->InAt(0).AsRegister<Register>();
+ bool is_volatile = field_info.IsVolatile();
+ DataType::Type field_type = field_info.GetFieldType();
+ uint32_t offset = field_info.GetFieldOffset().Uint32Value();
+
+ Address field_addr(base, offset);
+
+ HandleFieldSet(instruction,
+ /* value_index= */ 1,
+ field_type,
+ field_addr,
+ base,
+ is_volatile,
+ value_can_be_null);
+}
+
void LocationsBuilderX86::VisitStaticFieldGet(HStaticFieldGet* instruction) {
HandleFieldGet(instruction, instruction->GetFieldInfo());
}
diff --git a/compiler/optimizing/code_generator_x86.h b/compiler/optimizing/code_generator_x86.h
index 24993c3..efadae4 100644
--- a/compiler/optimizing/code_generator_x86.h
+++ b/compiler/optimizing/code_generator_x86.h
@@ -243,6 +243,14 @@
Label* fixup_label,
ReadBarrierOption read_barrier_option);
+ void HandleFieldSet(HInstruction* instruction,
+ uint32_t value_index,
+ DataType::Type type,
+ Address field_addr,
+ Register base,
+ bool is_volatile,
+ bool value_can_be_null);
+
private:
// Generate code for the given suspend check. If not null, `successor`
// is the block to branch to if the suspend check is not needed, and after
diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc
index 1c1b4ea..823b011 100644
--- a/compiler/optimizing/intrinsics_x86.cc
+++ b/compiler/optimizing/intrinsics_x86.cc
@@ -3428,7 +3428,7 @@
__ Bind(slow_path->GetExitLabel());
}
-void IntrinsicLocationsBuilderX86::VisitVarHandleSet(HInvoke* invoke) {
+static void CreateVarHandleSetLocations(HInvoke* invoke) {
// The only read barrier implementation supporting the
// VarHandleGet intrinsic is the Baker-style read barriers.
if (kEmitCompilerReadBarrier && !kUseBakerReadBarrier) {
@@ -3443,29 +3443,23 @@
uint32_t value_index = invoke->GetNumberOfArguments() - 1;
HInstruction* value = invoke->InputAt(value_index);
DataType::Type value_type = GetDataTypeFromShorty(invoke, value_index);
- if (value_type == DataType::Type::kInt64 && !value->IsConstant()) {
- // We avoid the case of a non-constant Int64 value because we would need to place it in a
- // register pair. If the slow path is taken, the ParallelMove might fail to move the pair
- // according to the X86DexCallingConvention in case of an overlap (e.g., move the int64 value
- // from <EAX, EBX> to <EBX, ECX>).
+ bool is_volatile = invoke->GetIntrinsic() == Intrinsics::kVarHandleSetVolatile;
+ if (value_type == DataType::Type::kInt64 && (!value->IsConstant() || is_volatile)) {
+ // We avoid the case of a non-constant (or volatile) Int64 value because we would need to
+ // place it in a register pair. If the slow path is taken, the ParallelMove might fail to move
+ // the pair according to the X86DexCallingConvention in case of an overlap (e.g., move the
+ // int64 value from <EAX, EBX> to <EBX, ECX>). (Bug: b/168687887)
return;
}
ArenaAllocator* allocator = invoke->GetBlock()->GetGraph()->GetAllocator();
LocationSummary* locations = new (allocator) LocationSummary(
invoke, LocationSummary::kCallOnSlowPath, kIntrinsified);
- locations->AddTemp(Location::RequiresRegister());
- // This temporary register is also used for card for MarkGCCard. Make sure it's a byte register
- locations->AddTemp(Location::RegisterLocation(EAX));
locations->SetInAt(0, Location::RequiresRegister());
size_t expected_coordinates_count = GetExpectedVarHandleCoordinatesCount(invoke);
if (expected_coordinates_count == 1u) {
// For instance fields, this is the source object
locations->SetInAt(1, Location::RequiresRegister());
- } else if (value_type == DataType::Type::kReference) {
- // For static reference fields, we need another temporary for MarkGCCard because one will
- // be busy with the declaring class.
- locations->AddTemp(Location::RequiresRegister());
}
switch (value_type) {
@@ -3481,7 +3475,7 @@
locations->SetInAt(value_index, Location::RegisterOrConstant(value));
break;
case DataType::Type::kInt64:
- // We only handle constant int64 values.
+ // We only handle constant non-volatile int64 values.
DCHECK(value->IsConstant());
locations->SetInAt(value_index, Location::ConstantLocation(value->AsConstant()));
break;
@@ -3490,16 +3484,29 @@
break;
default:
DCHECK(DataType::IsFloatingPointType(value_type));
- locations->SetInAt(value_index, Location::FpuRegisterOrConstant(value));
+ if (is_volatile && value_type == DataType::Type::kFloat64) {
+ locations->SetInAt(value_index, Location::RequiresFpuRegister());
+ } else {
+ locations->SetInAt(value_index, Location::FpuRegisterOrConstant(value));
+ }
+ }
+
+ locations->AddTemp(Location::RequiresRegister());
+ // This temporary register is also used for card for MarkGCCard. Make sure it's a byte register
+ locations->AddTemp(Location::RegisterLocation(EAX));
+ if (expected_coordinates_count == 0 && value_type == DataType::Type::kReference) {
+ // For static reference fields, we need another temporary for the declaring class. We set it
+ // last because we want to make sure that the first 2 temps are reserved for HandleFieldSet.
+ locations->AddTemp(Location::RequiresRegister());
}
}
-void IntrinsicCodeGeneratorX86::VisitVarHandleSet(HInvoke* invoke) {
+static void GenerateVarHandleSet(HInvoke* invoke, CodeGeneratorX86* codegen) {
// The only read barrier implementation supporting the
// VarHandleGet intrinsic is the Baker-style read barriers.
DCHECK(!kEmitCompilerReadBarrier || kUseBakerReadBarrier);
- X86Assembler* assembler = codegen_->GetAssembler();
+ X86Assembler* assembler = codegen->GetAssembler();
LocationSummary* locations = invoke->GetLocations();
// The value we want to set is the last argument
uint32_t value_index = invoke->GetNumberOfArguments() - 1;
@@ -3508,8 +3515,8 @@
Register varhandle_object = locations->InAt(0).AsRegister<Register>();
Register temp = locations->GetTemp(0).AsRegister<Register>();
Register temp2 = locations->GetTemp(1).AsRegister<Register>();
- SlowPathCode* slow_path = new (codegen_->GetScopedAllocator()) IntrinsicSlowPathX86(invoke);
- codegen_->AddSlowPath(slow_path);
+ SlowPathCode* slow_path = new (codegen->GetScopedAllocator()) IntrinsicSlowPathX86(invoke);
+ codegen->AddSlowPath(slow_path);
GenerateVarHandleCommonChecks(invoke, temp, slow_path, assembler);
@@ -3526,52 +3533,85 @@
Address(varhandle_object, var_type_offset),
slow_path,
assembler);
+
+ // For static reference fields, we need another temporary for the declaring class. But since
+ // for instance fields the object is in a separate register, it is safe to use the first
+ // temporary register for GenerateVarHandleFieldReference.
+ size_t expected_coordinates_count = GetExpectedVarHandleCoordinatesCount(invoke);
+ temp = expected_coordinates_count == 1u ? temp : locations->GetTemp(2).AsRegister<Register>();
}
Register offset = temp2;
// Get the field referred by the VarHandle. The returned register contains the object reference
// or the declaring class. The field offset will be placed in 'offset'. For static fields, the
// declaring class will be placed in 'temp' register.
- Register reference = GenerateVarHandleFieldReference(invoke, codegen_, temp, offset);
+ Register reference = GenerateVarHandleFieldReference(invoke, codegen, temp, offset);
- // Store the value to the field
- CodeGeneratorX86* codegen_x86 = down_cast<CodeGeneratorX86*>(codegen_);
- if (value_type == DataType::Type::kReference) {
- size_t expected_coordinates_count = GetExpectedVarHandleCoordinatesCount(invoke);
- bool needs_write_barrier =
- CodeGenerator::StoreNeedsWriteBarrier(value_type, invoke->InputAt(value_index));
- Register value_reg = value.AsRegister<Register>();
- // We use the second temporary for the card to make sure it's a byte register
- Register card = temp2;
- // For static reference fields, we need another temporary for MarkGCCard. But since for
- // instance fields the object is in a separate register, it is safe to use the first
- // temporary register for the card.
- // It can be used for reference poisoning too.
- temp = expected_coordinates_count == 1u ? temp : locations->GetTemp(2).AsRegister<Register>();
-
- if (kPoisonHeapReferences && needs_write_barrier) {
- // Note that in the case where `value` is a null reference,
- // we do not enter this block, as the reference does not
- // need poisoning.
- __ movl(temp, value_reg);
- __ PoisonHeapReference(temp);
- __ movl(Address(reference, offset, TIMES_1, 0), temp);
- } else if (value.IsConstant()) {
- int32_t v = CodeGenerator::GetInt32ValueOf(value.GetConstant());
- __ movl(Address(reference, offset, TIMES_1, 0), Immediate(v));
- } else {
- __ movl(Address(reference, offset, TIMES_1, 0), value_reg);
- }
- if (needs_write_barrier) {
- codegen_->MarkGCCard(temp, card, reference, value_reg, false);
- }
- } else {
- codegen_x86->MoveToMemory(value_type, value, reference, offset);
+ bool is_volatile = false;
+ switch (invoke->GetIntrinsic()) {
+ case Intrinsics::kVarHandleSet:
+ case Intrinsics::kVarHandleSetOpaque:
+ // The only constraint for setOpaque is to ensure bitwise atomicity (atomically set 64 bit
+ // values), but we don't treat Int64 values because we would need to place it in a register
+ // pair. If the slow path is taken, the Parallel move might fail to move the register pair
+ // in case of an overlap (e.g., move from <EAX, EBX> to <EBX, ECX>). (Bug: b/168687887)
+ break;
+ case Intrinsics::kVarHandleSetVolatile:
+ is_volatile = true;
+ break;
+ case Intrinsics::kVarHandleSetRelease:
+ codegen->GenerateMemoryBarrier(MemBarrierKind::kAnyStore);
+ break;
+ default:
+ LOG(FATAL) << "GenerateVarHandleSet received non-set intrinsic " << invoke->GetIntrinsic();
}
+ InstructionCodeGeneratorX86* instr_codegen =
+ down_cast<InstructionCodeGeneratorX86*>(codegen->GetInstructionVisitor());
+ // Store the value to the field
+ instr_codegen->HandleFieldSet(invoke,
+ value_index,
+ value_type,
+ Address(reference, offset, TIMES_1, 0),
+ reference,
+ is_volatile,
+ /* value_can_be_null */ true);
+
__ Bind(slow_path->GetExitLabel());
}
+void IntrinsicLocationsBuilderX86::VisitVarHandleSet(HInvoke* invoke) {
+ CreateVarHandleSetLocations(invoke);
+}
+
+void IntrinsicCodeGeneratorX86::VisitVarHandleSet(HInvoke* invoke) {
+ GenerateVarHandleSet(invoke, codegen_);
+}
+
+void IntrinsicLocationsBuilderX86::VisitVarHandleSetVolatile(HInvoke* invoke) {
+ CreateVarHandleSetLocations(invoke);
+}
+
+void IntrinsicCodeGeneratorX86::VisitVarHandleSetVolatile(HInvoke* invoke) {
+ GenerateVarHandleSet(invoke, codegen_);
+}
+
+void IntrinsicLocationsBuilderX86::VisitVarHandleSetRelease(HInvoke* invoke) {
+ CreateVarHandleSetLocations(invoke);
+}
+
+void IntrinsicCodeGeneratorX86::VisitVarHandleSetRelease(HInvoke* invoke) {
+ GenerateVarHandleSet(invoke, codegen_);
+}
+
+void IntrinsicLocationsBuilderX86::VisitVarHandleSetOpaque(HInvoke* invoke) {
+ CreateVarHandleSetLocations(invoke);
+}
+
+void IntrinsicCodeGeneratorX86::VisitVarHandleSetOpaque(HInvoke* invoke) {
+ GenerateVarHandleSet(invoke, codegen_);
+}
+
static void CreateVarHandleCompareAndSetLocations(HInvoke* invoke) {
// The only read barrier implementation supporting the
// VarHandleGet intrinsic is the Baker-style read barriers.
@@ -3808,9 +3848,6 @@
UNIMPLEMENTED_INTRINSIC(X86, VarHandleGetAndSetRelease)
UNIMPLEMENTED_INTRINSIC(X86, VarHandleGetOpaque)
UNIMPLEMENTED_INTRINSIC(X86, VarHandleGetVolatile)
-UNIMPLEMENTED_INTRINSIC(X86, VarHandleSetOpaque)
-UNIMPLEMENTED_INTRINSIC(X86, VarHandleSetRelease)
-UNIMPLEMENTED_INTRINSIC(X86, VarHandleSetVolatile)
UNREACHABLE_INTRINSICS(X86)
diff --git a/compiler/utils/x86/assembler_x86.h b/compiler/utils/x86/assembler_x86.h
index 05345a7..c546927 100644
--- a/compiler/utils/x86/assembler_x86.h
+++ b/compiler/utils/x86/assembler_x86.h
@@ -73,6 +73,10 @@
return static_cast<Register>(encoding_at(1) & 7);
}
+ int32_t disp() const {
+ return disp_;
+ }
+
int8_t disp8() const {
CHECK_GE(length_, 2);
return static_cast<int8_t>(encoding_[length_ - 1]);
@@ -92,7 +96,7 @@
protected:
// Operand can be sub classed (e.g: Address).
- Operand() : length_(0), fixup_(nullptr) { }
+ Operand() : length_(0), disp_(0), fixup_(nullptr) { }
void SetModRM(int mod_in, Register rm_in) {
CHECK_EQ(mod_in & ~3, 0);
@@ -110,6 +114,7 @@
void SetDisp8(int8_t disp) {
CHECK(length_ == 1 || length_ == 2);
encoding_[length_++] = static_cast<uint8_t>(disp);
+ disp_ = disp;
}
void SetDisp32(int32_t disp) {
@@ -117,6 +122,7 @@
int disp_size = sizeof(disp);
memmove(&encoding_[length_], &disp, disp_size);
length_ += disp_size;
+ disp_ = disp;
}
AssemblerFixup* GetFixup() const {
@@ -130,12 +136,13 @@
private:
uint8_t length_;
uint8_t encoding_[6];
+ int32_t disp_;
// A fixup can be associated with the operand, in order to be applied after the
// code has been generated. This is used for constant area fixups.
AssemblerFixup* fixup_;
- explicit Operand(Register reg) : fixup_(nullptr) { SetModRM(3, reg); }
+ explicit Operand(Register reg) : disp_(0), fixup_(nullptr) { SetModRM(3, reg); }
// Get the operand encoding byte at the given index.
uint8_t encoding_at(int index_in) const {
@@ -191,6 +198,15 @@
SetFixup(fixup);
}
+ Address displaceBy(int offset) {
+ if (rm() == ESP) {
+ // SIB addressing mode
+ return Address(base(), index(), scale(), disp() + offset, GetFixup());
+ } else {
+ return Address(rm(), disp() + offset, GetFixup());
+ }
+ }
+
static Address Absolute(uintptr_t addr) {
Address result;
result.SetModRM(0, EBP);