-
Bug
-
Resolution: Duplicate
-
P4
-
19
-
x86
x86 defines r10 and r11 to be rscratch1 and rscratch2, respectively. These are, however, misleading, because there is no scratch register on x86, as r10 and r11 are both legal candidates for register allocation.
This leads to real bugs in ad file, for example in absF_reg_reg, the result is obtained by anding the argument with 0x80000000, this value is stored in an external stub. As a result, in compile time, the address may be unreachable using rip relative address mode and must be materialised on a register. MacroAssembler::vandps(XMMRegister, XMMRegister, AddressLiteral, int, Register) may kill a scratch register that default to r10. The ad file, however, is unaware of this and happily calls that function with the default argument, risks convoluting r10.
I can actually reproduce a crash with this bug, the details are at the end of the description. As a result, it may be desirable to remove rscratch1 and rscratch2 from x86 assembler definition to avoid such bugs and confusion.
The crash can be observed on a slight modification of hotspot that forces materialisation of the external address on a JMH benchmark (for blackhole usage only)
--- a/src/hotspot/cpu/x86/macroAssembler_x86.cpp
+++ b/src/hotspot/cpu/x86/macroAssembler_x86.cpp
@@ -3386,7 +3386,7 @@ void MacroAssembler::vandpd(XMMRegister dst, XMMRegister nds, AddressLiteral src
}
void MacroAssembler::vandps(XMMRegister dst, XMMRegister nds, AddressLiteral src, int vector_len, Register scratch_reg) {
- if (reachable(src)) {
+ if (false) {
vandps(dst, nds, as_Address(src), vector_len);
} else {
lea(scratch_reg, src);
on this function:
record Point(int x, int y) {}
@Benchmark
public void test(Blackhole bh) {
Point x0 = x[0], x1 = x[1], x2 = x[2], x3 = x[3], x4 = x[4], x5 = x[5], x6 = x[6], x7 = x[7];
Point x8 = x[8], x9 = x[9], x10 = x[10], x11 = x[11], x12 = x[12], x13 = x[13], x14 = x[14], x15 = x[15];
int sum = x0.x + x1.x + x2.x + x3.x + x4.x + x5.x + x6.x + x7.x + x8.x + x9.x + x10.x + x11.x + x12.x + x13.x + x14.x + x15.x;
sum = (int)Math.abs((float)sum);
sum += x0.y; bh.consume(sum);
sum += x1.y; bh.consume(sum);
sum += x2.y; bh.consume(sum);
sum += x3.y; bh.consume(sum);
sum += x4.y; bh.consume(sum);
sum += x5.y; bh.consume(sum);
sum += x6.y; bh.consume(sum);
sum += x7.y; bh.consume(sum);
sum += x8.y; bh.consume(sum);
sum += x9.y; bh.consume(sum);
sum += x10.y; bh.consume(sum);
sum += x11.y; bh.consume(sum);
sum += x12.y; bh.consume(sum);
sum += x13.y; bh.consume(sum);
sum += x14.y; bh.consume(sum);
sum += x15.y; bh.consume(sum);
bh.consume(sum);
}
This leads to real bugs in ad file, for example in absF_reg_reg, the result is obtained by anding the argument with 0x80000000, this value is stored in an external stub. As a result, in compile time, the address may be unreachable using rip relative address mode and must be materialised on a register. MacroAssembler::vandps(XMMRegister, XMMRegister, AddressLiteral, int, Register) may kill a scratch register that default to r10. The ad file, however, is unaware of this and happily calls that function with the default argument, risks convoluting r10.
I can actually reproduce a crash with this bug, the details are at the end of the description. As a result, it may be desirable to remove rscratch1 and rscratch2 from x86 assembler definition to avoid such bugs and confusion.
The crash can be observed on a slight modification of hotspot that forces materialisation of the external address on a JMH benchmark (for blackhole usage only)
--- a/src/hotspot/cpu/x86/macroAssembler_x86.cpp
+++ b/src/hotspot/cpu/x86/macroAssembler_x86.cpp
@@ -3386,7 +3386,7 @@ void MacroAssembler::vandpd(XMMRegister dst, XMMRegister nds, AddressLiteral src
}
void MacroAssembler::vandps(XMMRegister dst, XMMRegister nds, AddressLiteral src, int vector_len, Register scratch_reg) {
- if (reachable(src)) {
+ if (false) {
vandps(dst, nds, as_Address(src), vector_len);
} else {
lea(scratch_reg, src);
on this function:
record Point(int x, int y) {}
@Benchmark
public void test(Blackhole bh) {
Point x0 = x[0], x1 = x[1], x2 = x[2], x3 = x[3], x4 = x[4], x5 = x[5], x6 = x[6], x7 = x[7];
Point x8 = x[8], x9 = x[9], x10 = x[10], x11 = x[11], x12 = x[12], x13 = x[13], x14 = x[14], x15 = x[15];
int sum = x0.x + x1.x + x2.x + x3.x + x4.x + x5.x + x6.x + x7.x + x8.x + x9.x + x10.x + x11.x + x12.x + x13.x + x14.x + x15.x;
sum = (int)Math.abs((float)sum);
sum += x0.y; bh.consume(sum);
sum += x1.y; bh.consume(sum);
sum += x2.y; bh.consume(sum);
sum += x3.y; bh.consume(sum);
sum += x4.y; bh.consume(sum);
sum += x5.y; bh.consume(sum);
sum += x6.y; bh.consume(sum);
sum += x7.y; bh.consume(sum);
sum += x8.y; bh.consume(sum);
sum += x9.y; bh.consume(sum);
sum += x10.y; bh.consume(sum);
sum += x11.y; bh.consume(sum);
sum += x12.y; bh.consume(sum);
sum += x13.y; bh.consume(sum);
sum += x14.y; bh.consume(sum);
sum += x15.y; bh.consume(sum);
bh.consume(sum);
}
- duplicates
-
JDK-8292878 x86: Make scratch register usage explicit in assembler code
-
- Resolved
-