[15] RFR(M): 8242108: Performance regression after fix for JDK-8229496

Eric Liu eric.c.liu at arm.com
Tue Apr 21 04:12:33 UTC 2020


Hi Tobias,

I'm not sure whether you noticed that with https://bugs.openjdk.java.net/browse/JDK-8229496, the 'CastII/CastLL' would mislead GVN that make it unable to recognize the same 'CmpNode' as before.

E.g. for java code:
```
        public int foo(int a, int b) {
            int r = a / b;
            r = r / b; // no need zero-check
            r = r / b; // no need zero-check
            return r;
        }
```

The zero-check for 'b' could not be removed as before if 'b' is boxed with CastII.

I think backing out the original fix for 8229496 would solve this problem.


One comment:

The test case [1] try to detect the wrong dependency order but I assume it unable to find the same issue in AArch64 due to the different behavior of div/mod compared with AMD64.
 
[1] http://cr.openjdk.java.net/~thartmann/8229496/webrev.00/raw_files/new/test/hotspot/jtreg/compiler/loopopts/TestDivZeroCheckControl.java 


Thanks,
Eric

-----Original Message-----
From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> On Behalf Of Tobias Hartmann
Sent: Monday, April 20, 2020 11:52 PM
To: hotspot compiler <hotspot-compiler-dev at openjdk.java.net>
Subject: [15] RFR(M): 8242108: Performance regression after fix for JDK-8229496

Hi,

please review the following patch:
https://bugs.openjdk.java.net/browse/JDK-8242108
http://cr.openjdk.java.net/~thartmann/8242108/webrev.00/

The fix for 8229496 [1] triggers a performance regression with NumberFormat.format(). The problem is the additional control dependency on a CastII/LL which restricts optimizations due to _carry_dependency being set (which was necessary because we can not represent non-null integers/long values in C2's type system).

While investigating, I've noticed that Roland's fix for 8241900 [2] fixes the exact same problem but in a more elegant way, avoiding an impact on performance.

I'm therefore proposing to back out the original fix for 8229496, leaving the regression test in and also adding a microbenchmark. I've verified that this solves the performance regression (4547 ops/ms vs. 5048 ops/ms on my machine).

Thanks,
Tobias

[1] https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-August/034865.html
[2] https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-April/037778.html


More information about the hotspot-compiler-dev mailing list