RFR: 8297689: Fix incorrect result of Short.reverseBytes() call in loops

Pengfei Li pli at openjdk.org
Wed Nov 30 07:29:22 UTC 2022


Recently, we find calling `Short.reverseBytes()` in loops may generate incorrect result if the code is compiled by C2. Below is a simple case to reproduce.


class Foo {
  static final int SIZE = 50;
  static int a[] = new int[SIZE];

  static void test() {
    for (int i = 0; i < SIZE; i++) {
      a[i] = Short.reverseBytes((short) a[i]);
    }
  }

  public static void main(String[] args) throws Exception {
    Class.forName("java.lang.Short");
    a[25] = 16;
    test();
    System.out.println(a[25]);
  }
}

// $ java -Xint Foo
// 4096
// $ java -Xcomp -XX:-TieredCompilation -XX:CompileOnly=Foo.test Foo
// 268435456


In this case, the `reverseBytes()` call is intrinsified and transformed into a `ReverseBytesS` node. But then C2 compiler incorrectly vectorizes it into `ReverseBytesV` with int type. C2 `Op_ReverseBytes*` has short, char, int and long versions. Their behaviors are different for different data sizes. In superword, subword operation itself doesn't have precise data size info. Instead, the data size info comes from memory operations in its use-def chain. Hence, vectorization of `reverseBytes()` is valid only if the data size is consistent with the type size of the caller's class. But current C2 compiler code lacks fine-grained type checks for `ReverseBytes*` in vector transformation. It results in `reverseBytes()` call from Short or Character class with int load/store gets vectorized incorrectly in above case.

To fix the issue, this patch adds more checks in `VectorNode::opcode()`. T_BYTE is a special case for `Op_ReverseBytes*`. As the Java Byte class doesn't have `reverseBytes()` method so there's no `Op_ReverseBytesB`. But T_BYTE may still appear in VectorAPI calls. In this patch we still use `Op_ReverseBytesI` for T_BYTE to ensure vector intrinsification succeeds.

Tested with hotspot::hotspot_all_no_apps, jdk tier1~3 and langtools tier1 on x86 and AArch64, no issue is found.

-------------

Commit messages:
 - 8297689: Fix incorrect result of Short.reverseBytes() call in loops

Changes: https://git.openjdk.org/jdk/pull/11427/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11427&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8297689
  Stats: 166 lines in 6 files changed: 160 ins; 2 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/11427.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/11427/head:pull/11427

PR: https://git.openjdk.org/jdk/pull/11427


More information about the hotspot-dev mailing list