RFR: 8328938: C2 SuperWord: disable vectorization for large stride and scale [v4]

Emanuel Peter epeter at openjdk.org
Tue Apr 2 06:28:18 UTC 2024


> **Problem**
> In [JDK-8310190](https://bugs.openjdk.org/browse/JDK-8310190) / https://git.openjdk.org/jdk/pull/14785 I fixed the alignment with `AlignVector`. For that, I had to compute `abs(scale)` and `abs(stride)`, as well as `scale * stride`. The issue is that all of these values can overflow the int range (e.g. `abs(min_int) = min_int`).
> 
> We hit asserts like:
> 
> `#  assert(is_power_of_2(value)) failed: value must be a power of 2: 0xffffffff80000000`
> Happens because we take `abs(min_int)`, which is `min_int = 0x80000000`, and assuming this was a positive (unsigned) number is a power of 2 `2^31`. We then expand it to `long`, get `0xffffffff80000000`, which is not a power of 2 anymore. This violates the implicit assumptions, and we hit the assert.
> 
> `#  assert(q >= 1) failed: modulo value must be large enough`
> We have `scale = 2^30` and `stride = 4 = 2^2`. For the alignment calculation we compute `scale * stride = 2^32`, which overflows the int range and becomes zero.
> 
> Before [JDK-8310190](https://bugs.openjdk.org/browse/JDK-8310190) we could get similar issues with the (old) code in `SuperWord::ref_is_alignable`, if `AlignVector` is enabled:
> 
> 
> int span = preloop_stride * p.scale_in_bytes();
> ...
> if (vw % span == 0) {
> 
> 
> if `span == 0` because of overflow, then the `idiv` from the modulo gets a division by zero -> `SIGFPE`.
> 
> But it seems the bug is possibly a regression from JDK20 b2 [JDK-8286197](https://bugs.openjdk.org/browse/JDK-8286197). Here we enabled certaint Unsafe memory access address patterns, and it is such patterns that the reproducer requires.
> 
> **Solution**
> I could either patch up all the code that works with `scale` and `stride`, and make sure no overflows ever happen. But that is quite involved and error prone.
> 
> I now just disable vectorization for large `scale` and `stride`. This should not have any performance impact, because such large `scale` and `stride` would lead to highly inefficient memory accesses, since they are spaced very far apart.

Emanuel Peter has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:

 - Merge branch 'JDK-8328938-abs-min-int-assert' of https://github.com/eme64/jdk into JDK-8328938-abs-min-int-assert
 - Apply suggestions from code review
   
   Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
 - Merge branch 'master' into JDK-8328938-abs-min-int-assert
 - improve comments
 - 8328938

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/18485/files
  - new: https://git.openjdk.org/jdk/pull/18485/files/9f8ed495..6f60fdf5

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18485&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18485&range=02-03

  Stats: 369085 lines in 3154 files changed: 23903 ins; 19414 del; 325768 mod
  Patch: https://git.openjdk.org/jdk/pull/18485.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18485/head:pull/18485

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


More information about the hotspot-compiler-dev mailing list