[optimizing] Address x86_64 RIP patch comments
Nicolas had some comments after the patch
https://android-review.googlesource.com/#/c/144100 had merged. Fix the
problems that he found.
Change-Id: I40e8a4273997860db7511dc8f1986281b72bead2
Signed-off-by: Mark Mendell <mark.p.mendell@intel.com>
diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc
index c915b0f..5710ec5 100644
--- a/compiler/optimizing/code_generator_x86_64.cc
+++ b/compiler/optimizing/code_generator_x86_64.cc
@@ -4234,13 +4234,14 @@
void CodeGeneratorX86_64::Finalize(CodeAllocator* allocator) {
// Generate the constant area if needed.
- if (!__ IsConstantAreaEmpty()) {
+ X86_64Assembler* assembler = GetAssembler();
+ if (!assembler->IsConstantAreaEmpty()) {
// Align to 4 byte boundary to reduce cache misses, as the data is 4 and 8
// byte values. If used for vectors at a later time, this will need to be
// updated to 16 bytes with the appropriate offset.
- __ Align(4, 0);
- constant_area_start_ = __ CodeSize();
- __ AddConstantArea();
+ assembler->Align(4, 0);
+ constant_area_start_ = assembler->CodeSize();
+ assembler->AddConstantArea();
}
// And finish up.
@@ -4252,7 +4253,7 @@
*/
class RIPFixup : public AssemblerFixup, public ArenaObject<kArenaAllocMisc> {
public:
- RIPFixup(CodeGeneratorX86_64& codegen, int offset)
+ RIPFixup(const CodeGeneratorX86_64& codegen, int offset)
: codegen_(codegen), offset_into_constant_area_(offset) {}
private:
@@ -4266,7 +4267,7 @@
region.StoreUnaligned<int32_t>(pos - 4, relative_position);
}
- CodeGeneratorX86_64& codegen_;
+ const CodeGeneratorX86_64& codegen_;
// Location in constant area that the fixup refers to.
int offset_into_constant_area_;
diff --git a/compiler/optimizing/code_generator_x86_64.h b/compiler/optimizing/code_generator_x86_64.h
index c819eec..aae7de0 100644
--- a/compiler/optimizing/code_generator_x86_64.h
+++ b/compiler/optimizing/code_generator_x86_64.h
@@ -297,7 +297,7 @@
X86_64Assembler assembler_;
const X86_64InstructionSetFeatures& isa_features_;
- // Offset to start of the constant area in the assembled code.
+ // Offset to the start of the constant area in the assembled code.
// Used for fixups to the constant area.
int constant_area_start_;
diff --git a/compiler/optimizing/intrinsics_x86_64.cc b/compiler/optimizing/intrinsics_x86_64.cc
index c0c4ff3..cbf94f0 100644
--- a/compiler/optimizing/intrinsics_x86_64.cc
+++ b/compiler/optimizing/intrinsics_x86_64.cc
@@ -301,15 +301,19 @@
locations->AddTemp(Location::RequiresFpuRegister()); // FP reg to hold mask.
}
-static void MathAbsFP(LocationSummary* locations, bool is64bit,
- X86_64Assembler* assembler, CodeGeneratorX86_64* codegen) {
+static void MathAbsFP(LocationSummary* locations,
+ bool is64bit,
+ X86_64Assembler* assembler,
+ CodeGeneratorX86_64* codegen) {
Location output = locations->Out();
if (output.IsFpuRegister()) {
// In-register
XmmRegister xmm_temp = locations->GetTemp(0).AsFpuRegister<XmmRegister>();
- // TODO: Can mask directly with constant area if we align on 16 bytes.
+ // TODO: Can mask directly with constant area using pand if we can guarantee
+ // that the literal is aligned on a 16 byte boundary. This will avoid a
+ // temporary.
if (is64bit) {
__ movsd(xmm_temp, codegen->LiteralInt64Address(INT64_C(0x7FFFFFFFFFFFFFFF)));
__ andpd(output.AsFpuRegister<XmmRegister>(), xmm_temp);
@@ -397,8 +401,11 @@
GenAbsInteger(invoke->GetLocations(), true, GetAssembler());
}
-static void GenMinMaxFP(LocationSummary* locations, bool is_min, bool is_double,
- X86_64Assembler* assembler, CodeGeneratorX86_64* codegen) {
+static void GenMinMaxFP(LocationSummary* locations,
+ bool is_min,
+ bool is_double,
+ X86_64Assembler* assembler,
+ CodeGeneratorX86_64* codegen) {
Location op1_loc = locations->InAt(0);
Location op2_loc = locations->InAt(1);
Location out_loc = locations->Out();
diff --git a/compiler/utils/x86_64/assembler_x86_64.cc b/compiler/utils/x86_64/assembler_x86_64.cc
index cb6d400..638659d 100644
--- a/compiler/utils/x86_64/assembler_x86_64.cc
+++ b/compiler/utils/x86_64/assembler_x86_64.cc
@@ -2742,14 +2742,14 @@
void X86_64Assembler::AddConstantArea() {
const std::vector<int32_t>& area = constant_area_.GetBuffer();
- for (size_t i = 0, u = area.size(); i < u; i++) {
+ for (size_t i = 0, e = area.size(); i < e; i++) {
AssemblerBuffer::EnsureCapacity ensured(&buffer_);
EmitInt32(area[i]);
}
}
int ConstantArea::AddInt32(int32_t v) {
- for (size_t i = 0, u = buffer_.size(); i < u; i++) {
+ for (size_t i = 0, e = buffer_.size(); i < e; i++) {
if (v == buffer_[i]) {
return i * elem_size_;
}
@@ -2766,8 +2766,8 @@
int32_t v_high = v >> 32;
if (buffer_.size() > 1) {
// Ensure we don't pass the end of the buffer.
- for (size_t i = 0, u = buffer_.size() - 1; i < u; i++) {
- if (v_low == buffer_[i] && v_high == buffer_[i+1]) {
+ for (size_t i = 0, e = buffer_.size() - 1; i < e; i++) {
+ if (v_low == buffer_[i] && v_high == buffer_[i + 1]) {
return i * elem_size_;
}
}
diff --git a/compiler/utils/x86_64/assembler_x86_64.h b/compiler/utils/x86_64/assembler_x86_64.h
index ef6205c..15b8b15 100644
--- a/compiler/utils/x86_64/assembler_x86_64.h
+++ b/compiler/utils/x86_64/assembler_x86_64.h
@@ -235,6 +235,8 @@
result.SetSIB(TIMES_1, CpuRegister(RSP), CpuRegister(RBP));
result.SetDisp32(addr);
} else {
+ // RIP addressing is done using RBP as the base register.
+ // The value in RBP isn't used. Instead the offset is added to RIP.
result.SetModRM(0, CpuRegister(RBP));
result.SetDisp32(addr);
}
@@ -244,6 +246,8 @@
// An RIP relative address that will be fixed up later.
static Address RIP(AssemblerFixup* fixup) {
Address result;
+ // RIP addressing is done using RBP as the base register.
+ // The value in RBP isn't used. Instead the offset is added to RIP.
result.SetModRM(0, CpuRegister(RBP));
result.SetDisp32(0);
result.SetFixup(fixup);
@@ -267,32 +271,20 @@
public:
ConstantArea() {}
- /**
- * Add a double to the constant area.
- * @param v literal to be added to the constant area.
- * @returns the offset in the constant area where the literal resides.
- */
+ // Add a double to the constant area, returning the offset into
+ // the constant area where the literal resides.
int AddDouble(double v);
- /**
- * Add a float to the constant area.
- * @param v literal to be added to the constant area.
- * @returns the offset in the constant area where the literal resides.
- */
+ // Add a float to the constant area, returning the offset into
+ // the constant area where the literal resides.
int AddFloat(float v);
- /**
- * Add an int32_t to the constant area.
- * @param v literal to be added to the constant area.
- * @returns the offset in the constant area where the literal resides.
- */
+ // Add an int32_t to the constant area, returning the offset into
+ // the constant area where the literal resides.
int AddInt32(int32_t v);
- /**
- * Add an int64_t to the constant area.
- * @param v literal to be added to the constant area.
- * @returns the offset in the constant area where the literal resides.
- */
+ // Add an int64_t to the constant area, returning the offset into
+ // the constant area where the literal resides.
int AddInt64(int64_t v);
int GetSize() const {
@@ -736,43 +728,26 @@
// and branch to a ExceptionSlowPath if it is.
void ExceptionPoll(ManagedRegister scratch, size_t stack_adjust) OVERRIDE;
- /**
- * Add a double to the constant area.
- * @param v literal to be added to the constant area.
- * @returns the offset in the constant area where the literal resides.
- */
+ // Add a double to the constant area, returning the offset into
+ // the constant area where the literal resides.
int AddDouble(double v) { return constant_area_.AddDouble(v); }
- /**
- * Add a float to the constant area.
- * @param v literal to be added to the constant area.
- * @returns the offset in the constant area where the literal resides.
- */
+ // Add a float to the constant area, returning the offset into
+ // the constant area where the literal resides.
int AddFloat(float v) { return constant_area_.AddFloat(v); }
- /**
- * Add an int32_t to the constant area.
- * @param v literal to be added to the constant area.
- * @returns the offset in the constant area where the literal resides.
- */
+ // Add an int32_t to the constant area, returning the offset into
+ // the constant area where the literal resides.
int AddInt32(int32_t v) { return constant_area_.AddInt32(v); }
- /**
- * Add an int64_t to the constant area.
- * @param v literal to be added to the constant area.
- * @returns the offset in the constant area where the literal resides.
- */
+ // Add an int64_t to the constant area, returning the offset into
+ // the constant area where the literal resides.
int AddInt64(int64_t v) { return constant_area_.AddInt64(v); }
- /**
- * Add the contents of the constant area to the assembler buffer.
- */
+ // Add the contents of the constant area to the assembler buffer.
void AddConstantArea();
- /**
- * Is the constant area empty?
- * @returns 'true' if there are no literals in the constant area.
- */
+ // Is the constant area empty? Return true if there are no literals in the constant area.
bool IsConstantAreaEmpty() const { return constant_area_.GetSize() == 0; }
private: