RFR: 8286104: use aggressive liveness for unstable_if traps [v11]
Vladimir Kozlov
kvn at openjdk.java.net
Mon Jun 6 20:42:24 UTC 2022
On Sun, 5 Jun 2022 22:59:13 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 one additional commit since the last revision:
>
> Bail out if fold-compares sees that a unstable_if trap has modified.
>
> Also add a regression test
Update is good. I agree with avoiding folding if unc trap info was modified.
I have few comments.
src/hotspot/share/opto/compile.cpp line 1911:
> 1909:
> 1910: void Compile::preprocess_unstable_if_traps() {
> 1911: #ifndef PRODUCT
You can use `#ifndef PRODUCT` around all this code and use `PRODUCT_RETURN` macro in header file:
`void preprocess_unstable_if_traps() PRODUCT_RETURN;` to het the same effect.
src/hotspot/share/opto/compile.cpp line 1952:
> 1950: const MethodLivenessResult& live_locals = method->liveness_at_bci(next_bci);
> 1951: assert(live_locals.is_valid(), "broken liveness info");
> 1952: bool changed = false;
Rename `changed` -> `modified`
src/hotspot/share/opto/compile.cpp line 1961:
> 1959: uint idx = jvms->locoff() + i;
> 1960: #ifndef PRODUCT
> 1961: if (Verbose) {
`Verbose` is debug (develop) flag. Use `#ifdef ASSERT` here.
src/hotspot/share/opto/compile.hpp line 811:
> 809: _dead_node_count = 0;
> 810: }
> 811: void record_unstable_if(UnstableIfTrap* trap);
You missed to rename method to record_unstable_if_trap().
The placement of declaration is strange. Can you move to other `unstable_if_trap` declared methods?
-------------
PR: https://git.openjdk.java.net/jdk/pull/8545
More information about the hotspot-compiler-dev
mailing list