RFR: 8320330: Improve implementation of RShift Value

[Tobias Hartmann]

On Mon, 20 Nov 2023 14:12:01 GMT, Jasmine Karthikeyan <jkarthikeyan at openjdk.org> wrote:

> 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!

Nice enhancement! Looks good to me.

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

Marked as reviewed by thartmann (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16736#pullrequestreview-1749726534