RFR: 8286104: use aggressive liveness for unstable_if traps [v2]

[Tobias Hartmann]

On Thu, 12 May 2022 21:27:30 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> I found that some phi nodes are useful because its uses are uncommon_trap nodes. In worse case, it would hinder boxing object eliminating and scalar replacement. Test.java of JDK-8286104 reveals this issue. This patch allows c2 parser to collect liveness based on next bci for unstable_if traps.  In most cases, next bci is closer to exits, so live locals are diminishing.  It helps to reduce the number of inputs of unstable_if traps. 
>> 
>> This is not a REDO of Optimization of Box nodes in uncommon_trap(JDK-8261137). Two patches are orthogonal.  I adapt test from [TestEliminateBoxInDebugInfo.java](https://github.com/openjdk/jdk/pull/2401/files#diff-49b2e38825aa4c28ca196bdc70c3cbecc2e835c2899f4f393527df4796b177ea), so part of credit goes to the original author.  I found that Scalar replacement can take care of the object `Integer ii = Integer.valueOf(value)` in original test even it can't be removed by later inliner.  I tweak the profiling data of Integer.valueOf() to hinder scalar replacement. 
>> 
>> This patch can cover the problem discovered by JDK-8261137. I ran the microbench and got 9x speedup on x86_64. It's almost same as JDK-8261137. 
>> 
>> Before:  
>> 
>> Benchmark               Mode  Cnt   Score   Error  Units
>> MyBenchmark.testMethod  avgt   10  32.776 ± 0.075  us/op
>> 
>> After: 
>> 
>> Benchmark               Mode  Cnt   Score   Error  Units
>> MyBenchmark.testMethod  avgt   10  3.656 ± 0.006  us/op
>>  ```
>> 
>> Testing
>> I ran tier1 test with and without `-XX:+DeoptimizeALot`. No regression has been found yet.
>
> Xin Liu has updated the pull request incrementally with 11 additional commits since the last revision:
> 
>  - revert code change from 1st revision.
>  - Merge branch 'JDK-8276998' into JDK-8286104
>  - rule out if a If nodes has 2 branches of unstable_if trap.
>  - change the flag to diagnostic.
>  - add sanity check for operands if bc is if_acmp_eq/ne and ifnull/nonnull
>  - fix release build
>  - update unstable_if after igvn.
>  - adjust unstable_if after fold_compares
>  - disable comparison_folding temporarily.
>    
>    This feature not only folds two CMPI but also merge two uncommon_traps.
>    it uses the dominating uncommon_trap and revaluate the two if in
>    interpreter.  currently, aggressiveliveness can't work for that.
>  - retain bci for unstable_if
>  - ... and 1 more: https://git.openjdk.java.net/jdk/compare/2c38b87b...2f047457

I ran this through some quick testing and `test/hotspot/jtreg/compiler/rangechecks/TestExplicitRangeChecks.java` fails:

java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:116)
	at java.base/java.lang.reflect.Method.invoke(Method.java:578)
	at compiler.rangechecks.TestExplicitRangeChecks.doTest(TestExplicitRangeChecks.java:441)
	at compiler.rangechecks.TestExplicitRangeChecks.main(TestExplicitRangeChecks.java:518)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	at java.base/java.lang.reflect.Method.invoke(Method.java:578)
	at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
	at java.base/java.lang.Thread.run(Thread.java:1585)
Caused by: java.lang.NullPointerException: Cannot read the array length because "<parameter2>" is null
	at compiler.rangechecks.TestExplicitRangeChecks.test3_2(TestExplicitRangeChecks.java:113)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	... 7 more

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

PR: https://git.openjdk.java.net/jdk/pull/8545