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