RFR: 8320330: Improve implementation of RShift Value

Jasmine Karthikeyan jkarthikeyan at openjdk.org
Mon Nov 20 15:05:19 UTC 2023


Hi, I've created this patch to improve the Value() implementation for right shift nodes. The primary change is preserving sign information when calculating the value, even if we can't come to a precise conclusion-- when looking at the right shift `X >> Y`, if `type(X)` is `>=0` then we can know that `type(X >> Y)` can be at most `>=0`, regardless of what `type(Y)` is. The inverse is also true for the `<0` case. Previously, these cases would have returned the entire int/long range as the set of possible values. I tried to find a more general way to approach this transform but the wrapping behavior of shifts made it hard to reason about and I didn't find many cases that would benefit from it, so I've decided to keep it simple.

I also slightly changed the logic that shifts the hi and lo ranges of the type to only operate on non-constant left-hand values, as the case where both are constant can be better handled by the end case that simply shifts the constant. The long variant has also been changed to better match the int version as well.

A common place to find this pattern is String#length:
https://github.com/openjdk/jdk/blob/db1d82347bb18e21c4c6a18076ffdaf17724c733/src/java.base/share/classes/java/lang/String.java#L1520-L1522

As it's shifting an array length, we should now be able to statically prove that the result will also be `>=0`. I ran some of the existing benchmarks and saw some nice improvements in the realm of string concatenation, due to the sharpened type allowing a branch to become dead:

                                                          Baseline                     Patch            Improvement
Benchmark                           (intValue) Mode   Cnt  Score   Error  Units    Score    Error  Units
StringConcat.concatConst4String        4711    avgt   15  23.471 ± 0.916  ns/op   21.630 ±  0.093  ns/op  (+ 8.16%)
StringConcat.concatConst6String        4711    avgt   15  29.386 ± 0.454  ns/op   28.292 ±  0.127  ns/op  (+ 3.79%)
StringConcat.concatConstString         4711    avgt   15   6.880 ± 0.051  ns/op    6.606 ±  0.021  ns/op  (+ 4.06%)
StringConcat.concatMethodConstString   4711    avgt   15   6.881 ± 0.029  ns/op    6.665 ±  0.069  ns/op  (+ 3.18%)


I've also attached an IR test, and tier1-3 testing passes on my linux x64 machine. Reviews would be appreciated!

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

Commit messages:
 - 8320330: Improve implementation of RShift Value

Changes: https://git.openjdk.org/jdk/pull/16736/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16736&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8320330
  Stats: 220 lines in 3 files changed: 209 ins; 6 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/16736.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16736/head:pull/16736

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


More information about the hotspot-compiler-dev mailing list