RFR: 8286104: use aggressive liveness for unstable_if traps

[Xin Liu]

On Fri, 6 May 2022 22:40:39 GMT, Vladimir Kozlov <kvn 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.
>
> Item 2. Yes, that is what will happened and that is why we may do this optimization. Your original words were confusing.
> 
> Again, MDO may not be update for rare case even after running in Interpreter for some time. As result recompiled code will be the same and we again hit unc trap.
> 
> In my additional comment I stated that placing uncommon trap to BC after merge point is wrong. You may not have all info in general cases (several branches merging to the same BC).

hi, @vnkozlov 
I try this idea with another approach. I move it from parser to optimizer, right after igvn. This approach keeps bci of uncommon_trap. 

I remember a speculative bci in IfNode when it has a stable_if.  One corner case is that 'fold-compares' of IfNode may share an uncommon_trap. The speculative bci would be wrong if this transformation occurs, so I drop this case. 

I also come up an idea to workaround the case that current bytecode eg. 'if_acmpeq' does reference scalarized objects. The operands are in stack of uncommon_trap's JVMState.  If a to-be-killed local variable is same as either lhs or rhs, just be conservative. 

@TobiHartmann 
With that configuration, C2 compiles the IRTest before the method is mature. Parser doesn't generate unstable_if at all. I remove uncommon_trap check from the test. 

thanks,
--lx

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

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