Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8284290

x86: Remove incorrectly defined scratch registers

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Open
    • Priority: P4
    • Resolution: Unresolved
    • Affects Version/s: 19
    • Fix Version/s: tbd
    • Component/s: hotspot

      Description

      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);
          }

        Attachments

          Activity

            People

            Assignee:
            qamai Quan Anh Mai
            Reporter:
            qamai Quan Anh Mai
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Dates

              Created:
              Updated: