8286104: use aggressive liveness for unstable_if traps #8545

navyxliu · 2022-05-05T05:30:06Z

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, 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. Besides runtime, the codecache utilization reduces from 1648 bytes to 1192 bytes, or 27.6%

Before:

Benchmark               Mode  Cnt   Score   Error  Units
MyBenchmark.testMethod  avgt   10  32.776 ± 0.075  us/op

Compiled method (c2)     281  636       4       MyBenchmark::testMethod (50 bytes)
 total in heap  [0x00007fa1e49ab510,0x00007fa1e49abb80] = 1648
 relocation     [0x00007fa1e49ab670,0x00007fa1e49ab6b0] = 64
 main code      [0x00007fa1e49ab6c0,0x00007fa1e49ab940] = 640
 stub code      [0x00007fa1e49ab940,0x00007fa1e49ab968] = 40
 oops           [0x00007fa1e49ab968,0x00007fa1e49ab978] = 16
 metadata       [0x00007fa1e49ab978,0x00007fa1e49ab990] = 24
 scopes data    [0x00007fa1e49ab990,0x00007fa1e49aba60] = 208
 scopes pcs     [0x00007fa1e49aba60,0x00007fa1e49abb30] = 208
 dependencies   [0x00007fa1e49abb30,0x00007fa1e49abb38] = 8
 handler table  [0x00007fa1e49abb38,0x00007fa1e49abb68] = 48
 nul chk table  [0x00007fa1e49abb68,0x00007fa1e49abb80] = 24

After:

Benchmark               Mode  Cnt   Score   Error  Units
MyBenchmark.testMethod  avgt   10  3.656 ± 0.006  us/op

Compiled method (c2)     288  633       4       MyBenchmark::testMethod (50 bytes)
 total in heap  [0x00007f35189ab010,0x00007f35189ab4b8] = 1192
 relocation     [0x00007f35189ab170,0x00007f35189ab1a0] = 48
 main code      [0x00007f35189ab1a0,0x00007f35189ab360] = 448
 stub code      [0x00007f35189ab360,0x00007f35189ab388] = 40
 oops           [0x00007f35189ab388,0x00007f35189ab390] = 8
 metadata       [0x00007f35189ab390,0x00007f35189ab398] = 8
 scopes data    [0x00007f35189ab398,0x00007f35189ab408] = 112
 scopes pcs     [0x00007f35189ab408,0x00007f35189ab488] = 128
 dependencies   [0x00007f35189ab488,0x00007f35189ab490] = 8
 handler table  [0x00007f35189ab490,0x00007f35189ab4a8] = 24
 nul chk table  [0x00007f35189ab4a8,0x00007f35189ab4b8] = 16

Testing
I ran tier1 test with and without -XX:+DeoptimizeALot. No regression has been found yet.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8286104: use aggressive liveness for unstable_if traps

Reviewers

Vladimir Kozlov (@vnkozlov - Reviewer)
Tobias Hartmann (@TobiHartmann - Reviewer) ⚠️ Review applies to 49bcc410

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/8545/head:pull/8545
$ git checkout pull/8545

Update a local copy of the PR:
$ git checkout pull/8545
$ git pull https://git.openjdk.org/jdk pull/8545/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 8545

View PR using the GUI difftool:
$ git pr show -t 8545

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/8545.diff

bridgekeeper · 2022-05-05T05:31:40Z

👋 Welcome back xliu! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2022-05-05T05:34:25Z

@navyxliu The following label will be automatically applied to this pull request:

hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2022-05-05T05:39:57Z

Webrevs

vnkozlov

Are you doing BCI and SP manipulation only to affect result of liveness_at_bci() call in kill_dead_locals()? May it can be done less disruptive way.
I am concern that different bci and sp may affect correctness of uncommon trap call generation (its debug info). It affects result of compute_stack_effects(), too_many_recompiles() and should_reexecute_implied_by_bytecode().
In addition logs about such uncommon trap will be different.

TobiHartmann · 2022-05-06T06:51:48Z

TestAggressiveLivenessForUnstableIf.java fails with -ea -esa -XX:CompileThreshold=100 -XX:+UnlockExperimentalVMOptions -server -XX:-TieredCompilation:

PrintIdeal:
 41  ConI  ===  0  [[ 42 ]]  #int:999999
 42  CmpI  === _  10  41  [[ 43 ]]  !jvms: TestAggressiveLivenessForUnstableIf::boxing_object @ bci:10 (line 51)
 40  ConI  ===  0  [[ 81 ]]  #int:0
 10  Parm  ===  3  [[ 81  42 ]] Parm0: int !jvms: TestAggressiveLivenessForUnstableIf::boxing_object @ bci:-1 (line 48)
 43  Bool  === _  42  [[ 81 ]] [le] !jvms: TestAggressiveLivenessForUnstableIf::boxing_object @ bci:10 (line 51)
 3  Start  ===  3  0  [[ 3  5  6  7  8  9  10 ]]  #{0:control, 1:abIO, 2:memory, 3:rawptr:BotPTR, 4:return_address, 5:int}
 81  CMoveI  === _  43  10  40  [[ 79 ]]  #int !orig=[78] !jvms: TestAggressiveLivenessForUnstableIf::boxing_object @ bci:20 (line 55)
 9  Parm  ===  3  [[ 79 ]] ReturnAdr !jvms: TestAggressiveLivenessForUnstableIf::boxing_object @ bci:-1 (line 48)
 8  Parm  ===  3  [[ 79 ]] FramePtr !jvms: TestAggressiveLivenessForUnstableIf::boxing_object @ bci:-1 (line 48)
 7  Parm  ===  3  [[ 79 ]] Memory  Memory: @BotPTR *+bot, idx=Bot; !jvms: TestAggressiveLivenessForUnstableIf::boxing_object @ bci:-1 (line 48)
 6  Parm  ===  3  [[ 79 ]] I_O !jvms: TestAggressiveLivenessForUnstableIf::boxing_object @ bci:-1 (line 48)
 5  Parm  ===  3  [[ 79 ]] Control !jvms: TestAggressiveLivenessForUnstableIf::boxing_object @ bci:-1 (line 48)
 79  Return  ===  5  6  7  8  9 returns 81  [[ 0 ]] 
 0  Root  ===  0  79  [[ 0  1  3  40  41 ]] inner 

Failed IR Rules (1) of Methods (1)
----------------------------------
1) Method "public static int compiler.c2.irTests.TestAggressiveLivenessForUnstableIf.boxing_object(int)" - [Failed IR rules: 1]:
   * @IR rule 1: "@compiler.lib.ir_framework.IR(applyIf={}, applyIfAnd={}, failOn={}, applyIfOr={}, counts={"(\\\\d+(\\\\s){2}(CallStaticJava.*)+(\\\\s){2}===.*uncommon_trap.*unstable_if)", "1"}, applyIfNot={})"
     - counts: Graph contains wrong number of nodes:
       * Regex 1: (\\d+(\\s){2}(CallStaticJava.*)+(\\s){2}===.*uncommon_trap.*unstable_if)
         - Failed comparison: [found] 0 = 1 [given]
         - No nodes matched!

navyxliu · 2022-05-06T19:03:08Z

hi, Vladimir and Tobias,

Thank you for reviewing this patch.

Are you doing BCI and SP manipulation only to affect result of liveness_at_bci() call in kill_dead_locals()? May it can be done less disruptive way.

yes, that is my goal. I tried the less disruptive way. One corner case is that the if bytecode may reference an object which happens to be dead at the next bci. It would be wrong if we avoid restoring it in deoptimization. As a result, I have to parse the if-bytecode.

Then I came up the idea that we can redefine the semantic of unstable_if trap, which is not part of JVM nor Java language specification. I change 'bci' of unstable_if trap from the if branch to the next bci. My assumption is as follows. I think they are both true.

the original bc has no side-effect.
reexecution in interpreter takes the other path, or next bci.

I am concern that different bci and sp may affect correctness of uncommon trap call generation (its debug info). It affects result of compute_stack_effects(), too_many_recompiles() and should_reexecute_implied_by_bytecode().
In addition logs about such uncommon trap will be different.

I want to change debuginfo. This example shows that we don't need to save the scalarized object in debuginfo. We don't need to repush arguments to stack(sp) for comparison either.

Both compute_stack_effects() and should_reexecute_implied_by_bytecode() are called by GraphKit::add_safepoint_edges(). we only call it when current bc() is associated with a function call or an allocation. I don't think if-family bytecodes need to call it.

There are 2 cases in Parse::do_if(). We do use PreserveJVMState to save both bci and sp for the taken branch, but we don't use PreserveJVMState for non-taken branch. I am truly concerned about it. so far, my explanation is that we only change bci for unstable_if, so stopped() has been true after that. Besides that, it's the last bytecode of a basic block, parser will reset bci when it processes the next basic block. What do you think about it?

navyxliu · 2022-05-06T21:11:49Z

hi, @vnkozlov
I agree with you that too_many_recompiles() is wrong in seems_stable_comparison. I will correct this.

bool Parse::seems_stable_comparison() const {
  if (C->too_many_traps(method(), bci(), Deoptimization::Reason_unstable_if)) {
    return false;
  }
  return true;
}

vnkozlov · 2022-05-06T21:44:55Z

Are you doing BCI and SP manipulation only to affect result of liveness_at_bci() call in kill_dead_locals()? May it can be done less disruptive way.

yes, that is my goal. I tried the less disruptive way. One corner case is that the if bytecode may reference an object which happens to be dead at the next bci. It would be wrong if we avoid restoring it in deoptimization. As a result, I have to parse the if-bytecode.

Got it. So the idea is that we don't need to re-materialize Object in case the path which will be taken after deoptimization in Interpreter will not use it. This seems reasonable optimization.

But I am still not sure that current implementation (shift uncommon trap to next bc) is valid.

Then I came up the idea that we can redefine the semantic of unstable_if trap, which is not part of JVM nor Java language specification. I change 'bci' of unstable_if trap from the if branch to the next bci. My assumption is as follows. I think they are both true.

the original bc has no side-effect.

reexecution in interpreter takes the other path, or next bci.

First, for your information we re-execute if to update its profiling information. For example, if it was some klass check we want to record new value in MDO. That is why we push arguments of if back on stack. New value can happen rarely and we may loose it if not recorded during such deoptimization.

I am not sure how your changes verify your assumptions.
And I am not clear what you mean in 2. Re-execution in Interpreter will be done according to information in uncommon trap (bci).

I am concern that different bci and sp may affect correctness of uncommon trap call generation (its debug info). It affects result of compute_stack_effects(), too_many_recompiles() and should_reexecute_implied_by_bytecode().
In addition logs about such uncommon trap will be different.

I want to change debuginfo. This example shows that we don't need to save the scalarized object in debuginfo. We don't need to repush arguments to stack(sp) for comparison either.

If you re-execute if you need its arguments on stack. I am not sure why you said we don't.

Both compute_stack_effects() and should_reexecute_implied_by_bytecode() are called by GraphKit::add_safepoint_edges(). we only call it when current bc() is associated with a function call or an allocation. I don't think if-family bytecodes need to call it.

add_safepoint_edges() is called for uncommon_trap which is generated as runtime call.
I agree that compute_stack_effects() may not be called because must_throw is false in this case.
But should_reexecute_implied_by_bytecode() could be called I think and you may get incorrect answer from Interpreter::bytecode_should_reexecute(code) because you changed BCI.

There are 2 cases in Parse::do_if(). We do use PreserveJVMState to save both bci and sp for the taken branch, but we don't use PreserveJVMState for non-taken branch. I am truly concerned about it. so far, my explanation is that we only change bci for unstable_if, so stopped() has been true after that. Besides that, it's the last bytecode of a basic block, parser will reset bci when it processes the next basic block. What do you think about it?

PreserveJVMState is used there only to provide correct starting JVM state for other branch processing. After merge JVM states from both branches are merged.

vnkozlov · 2022-05-06T22:11:42Z

Also in the test you pointed new uncommon trap is placed at bci 21 which is after merge point of branches.
Such uncommon trap should have information about merged values in debug info. And your changes do not provide that.
I think we need to limit this optimization to case when targeted BC is inside branch (if else code).

vnkozlov · 2022-05-06T22:30:24Z

Or not move uncommon trap - keep it at if bci. Which I prefer but it could be more complex changes as you said.

navyxliu · 2022-05-06T22:33:07Z

And I am not clear what you mean in 2. Re-execution in Interpreter will be done according to information in uncommon trap (bci).

first, I have to admit that I overlook the fact that reexecution will update MDO of if bytecode.

For item 2: reexecution in interpreter takes the other path, or next bci. Here is my thought: when HotSpot reinterprets the original bc, it must take the unstable path, otherwise this deoptimization shouldn't happen in the first place. In this patch, I just shift bci of uncommon_trap to next_bci, not only in IR, but also in ScopeDesc! Interpreter will start over with the shifted bci.

Here is what I generated for the unstable_if of Test::foo using this patch. please note that the bci changed from 11 to 21. The message pasted in JDK-8276998 was from my initial patch. I retained bci for uncommon_trap initially. I think this change is simpler.

02f     call,static  wrapper for: uncommon_trap(reason='unstable_if' action='reinterpret' debug_id='0')
        # Test::foo @ bci:21 (line 67) L[0]=_ L[1]=_ L[2]=#0
        # OopMap {off=52/0x34}
034     stop    # ShouldNotReachHere

I admit my change misses to update MDO in deoptimization. I think it's fine because HotSpot won't recompile this method again until interpreter evaluates it thousands of times. The MDO still gets updated.

vnkozlov · 2022-05-06T22:40:39Z

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).

navyxliu · 2022-05-06T23:00:03Z

hi, Vladimir,

I really appreciate you look into this issue. your inputs are super helpful.
I found that we have multiple options to attack Huawei's problem. I have yet another idea...

C2's speculative compilation is fascinating. The motive we would like to prune an infrequent branch should be that we can simplify IR and shrink code size. Back to Test::foo(), the else block is blank... Pruning a trivial basic block actually makes IR more complex. We should not replace a trivial basic block with unstable_if .

What Tobias reported is a great example. C2 compiled foo() before it became mature. therefore, C2 refrained his "heroic optimization. it turns out that c2 selects CMOVE and gets simpler, faster and smaller code...

thanks,
--lx

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.

openjdk · 2022-05-12T21:24:30Z

⚠️ @navyxliu This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

vnkozlov · 2022-06-03T18:33:08Z

I submitted new testing.

vnkozlov · 2022-06-03T20:48:30Z

2 tests failed so far. I put information into RFE.

vnkozlov · 2022-06-04T16:17:19Z

2 tests failed so far. I put information into RFE.

No other new failures in my tier1-7 testing. I think after you address found issue it will be ready to integrate (after second review by other Reviewer). But I would suggest to push it into JDK 20 after 19 is forked in one week to get more testing before release.

Also add a regression test

navyxliu · 2022-06-06T16:49:14Z

hi, @vnkozlov,

I file a new issue JDK-8287840(PR) for the new regression. It will allow some IfNodes carry out fold-compares in 1st IterGVN.

Back to this PR, for safety, I added a mechanism to guarantee that fold-compares gives up if its dominating trap has been modified. This is a guardrail and should not happen frequent.

I also add a regression test to cover fold-compare cases. for testEnumValues, c2 would prefer fold-compares if we merged JDK-8287840.

But I would suggest to push it into JDK 20 after 19 is forked in one week to get more testing before release.

sure. no problem.

thanks,
--lx

vnkozlov

Update is good. I agree with avoiding folding if unc trap info was modified.
I have few comments.

src/hotspot/share/opto/compile.hpp

src/hotspot/share/opto/compile.cpp

vnkozlov

Update looks good.

You need second review.

openjdk · 2022-06-13T23:29:56Z

@navyxliu This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8286104: use aggressive liveness for unstable_if traps

Reviewed-by: kvn, thartmann

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 123 new commits pushed to the master branch:

b6bd190: 8288985: P11TlsKeyMaterialGenerator should work with ChaCha20-Poly1305
ba670ec: 8289094: [JVMCI] reduce JNI overhead and other VM rounds trips in JVMCI
0709a6a: 8284942: Proxy building can just iterate superinterfaces once
2961b7e: 8285364: Remove REF_ enum for java.lang.ref.Reference
167ce4d: 8289421: No-PCH build for Minimal VM was broken by JDK-8287001
108cd69: 8283726: x86_64 intrinsics for compareUnsigned method in Integer and Long
b96ba19: 8289182: NMT: MemTracker::baseline should return void
779b4e1: 8287001: Add warning message when fail to load hsdis libraries
910053b: 8280235: Deprecated flag FlightRecorder missing from VMDeprecatedOptions test
7b3bf97: 8289401: Add dump output to TestRawRSACipher.java
... and 113 more: https://git.openjdk.org/jdk/compare/7039c6616ad1b67320f08e7ea4763e8ff4977430...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

TobiHartmann

Looks good overall. Some comments/questions:

Why can't we remove traps that have been modified?
I'm wondering how useful Compile::print_statistics() really is. Is it worth extending it? Is anyone using it?
Do you need to check for unstable if traps in Node::destruct?

src/hotspot/share/opto/parse.hpp

TobiHartmann · 2022-06-20T07:16:53Z

src/hotspot/share/opto/parse.hpp

+// Specialized uncommon_trap of unstable_if, we have 2 optimizations for them:
+//   1. suppress trivial Unstable_If traps
+//   2. use next_bci of _path to update live locals.
+class UnstableIfTrap {


What about moving this information into CallStaticJavaNode?

I think CallStaticJavaNode is popular. uncommon_trap/unstable_if is just a special case. that's why factor out and use a dedicated class for it.

TobiHartmann · 2022-06-20T07:24:51Z

src/hotspot/share/opto/compile.cpp

+    CallStaticJavaNode* unc = trap->uncommon_trap();
+    int next_bci = trap->next_bci();
+
+    if (next_bci != -1 && !trap->modified()) {


How can it be already modified? We are only processing each trap once, right?

Correct. there is only one pass right now. so it's redundant.

I discussed this with Vladimir Kozlov. Technically, we can run it after 'inline_incrementally()" as well. I think we can add it when we prove this optimization is stable. "!trap->modified()" can dedup for that.

src/hotspot/share/opto/parse.hpp

TobiHartmann · 2022-06-20T07:32:02Z

src/hotspot/share/opto/ifnode.cpp

@@ -838,7 +838,8 @@ bool IfNode::has_only_uncommon_traps(ProjNode* proj, ProjNode*& success, ProjNod
      ciMethod* dom_method = dom_unc->jvms()->method();
      int dom_bci = dom_unc->jvms()->bci();
      if (!igvn->C->too_many_traps(dom_method, dom_bci, Deoptimization::Reason_unstable_fused_if) &&
-          !igvn->C->too_many_traps(dom_method, dom_bci, Deoptimization::Reason_range_check)) {
+          !igvn->C->too_many_traps(dom_method, dom_bci, Deoptimization::Reason_range_check) &&
+          igvn->C->remove_unstable_if_trap(dom_unc)) {


This should be moved to IfNode::merge_uncommon_traps.

I tried that in the first place. It turns out that situation is complex. The following 3 functions take place in order.
has_only_uncommon_traps() => fold_compares_helper() => merge_uncommon_traps()

IfNode::has_only_uncommon_traps is the last stop before modifying code. if we moved the predicate "remove_unstable_if_trap(dom_unc)" to merge_uncommon_traps(), it would be too late when it returns true. We have to rollback code change in fold_compares_helper.

also changed the option from AggressiveLivenessForUnstableIf to OptimizeUnstableIf.

TobiHartmann

Thanks for the clarifications.

You added the same test twice with a slightly different name.

src/hotspot/share/opto/compile.cpp

test/hotspot/jtreg/compiler/c2/irTests/TestOptimizeUnstableIf.java

TobiHartmann · 2022-06-23T12:28:26Z

test/hotspot/jtreg/compiler/c2/irTests/TestOptimizeForUnstableIf.java

+/*
+ * @test
+ * @bug 8286104
+ * @summary Test C2 uses aggressive liveness to get rid of the boxing object which is


Suggested change

* @summary Test C2 uses aggressive liveness to get rid of the boxing object which is

* @summary Test that C2 uses aggressive liveness to get rid of the boxing object which is

src/hotspot/share/opto/parse.hpp

TobiHartmann · 2022-06-24T07:36:20Z

Looks good! Let me re-submit testing. I'll report back once it passed.

TobiHartmann

All tests passed.

navyxliu · 2022-06-27T17:18:08Z

hi, @vnkozlov
Could you take a look at the new revision? thanks,
--lx

vnkozlov

New changes are good except the change to flag.

src/hotspot/share/opto/c2_globals.hpp

src/hotspot/share/opto/ifnode.cpp

also update commments.

vnkozlov

Looks good. I ran it through tier1 to make sure this last change did not break it.

navyxliu · 2022-06-29T18:36:00Z

I take a look at the build failure on ubuntu/i386. The problem is that apt-get can't install gcc toolchains from a custom repo
[http://ppa.launchpad.net/ubuntu-toolchain-r/]. it's not caused by this PR.

vnkozlov · 2022-06-29T21:06:06Z

Good.

navyxliu · 2022-06-30T03:58:34Z

/integrate

openjdk · 2022-06-30T03:59:55Z

Going to push as commit 31e50f2.
Since your change was applied there have been 129 commits pushed to the master branch:

dddd4e7: 8289291: HttpServer sets incorrect value for "max" parameter in Keep-Alive header value
048bffa: Merge
cf71544: 8289252: Recommend Locale.of() method instead of the constructor
5708974: 8288596: Random:from() adapter does not delegate to supplied generator in all cases
dbc6e11: 8289399: Update SourceVersion to use snippets
15efb2b: 8289238: Refactoring changes to PassFailJFrame Test Framework
b6bd190: 8288985: P11TlsKeyMaterialGenerator should work with ChaCha20-Poly1305
ba670ec: 8289094: [JVMCI] reduce JNI overhead and other VM rounds trips in JVMCI
0709a6a: 8284942: Proxy building can just iterate superinterfaces once
2961b7e: 8285364: Remove REF_ enum for java.lang.ref.Reference
... and 119 more: https://git.openjdk.org/jdk/compare/7039c6616ad1b67320f08e7ea4763e8ff4977430...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-06-30T04:00:16Z

@navyxliu Pushed as commit 31e50f2.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Xin Liu added 2 commits May 5, 2022 05:26

8286104: use aggressive liveness for unstable_if traps

37de8fe

8286104: use aggressive liveness for unstable_if traps

2c38b87

openjdk bot added hotspot-compiler hotspot-compiler-dev@openjdk.org rfr Pull request is ready for review labels May 5, 2022

vnkozlov reviewed May 5, 2022

View reviewed changes

Xin Liu added 3 commits May 8, 2022 01:41

retain bci for unstable_if

c482800

disable comparison_folding temporarily.

2f650bf

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.

adjust unstable_if after fold_compares

8d9ba30

navyxliu marked this pull request as draft May 9, 2022 18:26

openjdk bot removed the rfr Pull request is ready for review label May 9, 2022

Xin Liu added 7 commits May 10, 2022 03:08

update unstable_if after igvn.

b4ceec5

fix release build

77ee4db

add sanity check for operands if bc is if_acmp_eq/ne and ifnull/nonnull

89fa3ce

change the flag to diagnostic.

5e689d4

rule out if a If nodes has 2 branches of unstable_if trap.

4f75dda

Merge branch 'JDK-8276998' into JDK-8286104

1b5cb89

revert code change from 1st revision.

2f04745

Bail out if fold-compares sees that a unstable_if trap has modified.

9c91737

Also add a regression test

vnkozlov reviewed Jun 6, 2022

View reviewed changes

src/hotspot/share/opto/compile.hpp Outdated Show resolved Hide resolved

src/hotspot/share/opto/compile.cpp Outdated Show resolved Hide resolved

src/hotspot/share/opto/compile.cpp Outdated Show resolved Hide resolved

src/hotspot/share/opto/compile.cpp Outdated Show resolved Hide resolved

monior change for code style.

81a8ccf

vnkozlov approved these changes Jun 13, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jun 13, 2022

TobiHartmann suggested changes Jun 20, 2022

View reviewed changes

Xin Liu added 2 commits June 20, 2022 23:17

Merge branch 'master' into JDK-8286104

481b66c

update per reviewer's feedback.

e5c8e55

also changed the option from AggressiveLivenessForUnstableIf to OptimizeUnstableIf.

TobiHartmann reviewed Jun 23, 2022

View reviewed changes

remove _path from UnstableIfTrap. remember _next_bci(int) is enough.

49bcc41

TobiHartmann approved these changes Jun 27, 2022

View reviewed changes

vnkozlov reviewed Jun 28, 2022

View reviewed changes

src/hotspot/share/opto/c2_globals.hpp Outdated Show resolved Hide resolved

src/hotspot/share/opto/ifnode.cpp Outdated Show resolved Hide resolved

restore the option OptimizeUnstableIf to diagnostic.

ed20a82

also update commments.

vnkozlov approved these changes Jun 29, 2022

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Jun 30, 2022

openjdk bot closed this Jun 30, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jun 30, 2022

	* @summary Test C2 uses aggressive liveness to get rid of the boxing object which is
	* @summary Test that C2 uses aggressive liveness to get rid of the boxing object which is

8286104: use aggressive liveness for unstable_if traps #8545

8286104: use aggressive liveness for unstable_if traps #8545

Conversation

navyxliu commented May 5, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented May 5, 2022

openjdk bot commented May 5, 2022

mlbridge bot commented May 5, 2022 • edited Loading

Webrevs

vnkozlov left a comment

Choose a reason for hiding this comment

TobiHartmann commented May 6, 2022

navyxliu commented May 6, 2022

navyxliu commented May 6, 2022

vnkozlov commented May 6, 2022

vnkozlov commented May 6, 2022 • edited Loading

vnkozlov commented May 6, 2022 • edited Loading

navyxliu commented May 6, 2022

vnkozlov commented May 6, 2022

navyxliu commented May 6, 2022 • edited Loading

openjdk bot commented May 12, 2022

vnkozlov commented Jun 3, 2022

vnkozlov commented Jun 3, 2022

vnkozlov commented Jun 4, 2022

navyxliu commented Jun 6, 2022 • edited Loading

vnkozlov left a comment

Choose a reason for hiding this comment

vnkozlov left a comment

Choose a reason for hiding this comment

openjdk bot commented Jun 13, 2022 • edited Loading

TobiHartmann left a comment

Choose a reason for hiding this comment

TobiHartmann Jun 20, 2022

Choose a reason for hiding this comment

navyxliu Jun 20, 2022

Choose a reason for hiding this comment

TobiHartmann Jun 20, 2022

Choose a reason for hiding this comment

navyxliu Jun 20, 2022

Choose a reason for hiding this comment

TobiHartmann Jun 20, 2022

Choose a reason for hiding this comment

navyxliu Jun 20, 2022

Choose a reason for hiding this comment

TobiHartmann left a comment

Choose a reason for hiding this comment

TobiHartmann Jun 23, 2022

Choose a reason for hiding this comment

TobiHartmann commented Jun 24, 2022

TobiHartmann left a comment

Choose a reason for hiding this comment

navyxliu commented Jun 27, 2022

vnkozlov left a comment

Choose a reason for hiding this comment

vnkozlov left a comment

Choose a reason for hiding this comment

navyxliu commented Jun 29, 2022

vnkozlov commented Jun 29, 2022

navyxliu commented Jun 30, 2022

openjdk bot commented Jun 30, 2022

openjdk bot commented Jun 30, 2022

navyxliu commented May 5, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented May 5, 2022 •

edited

Loading

vnkozlov commented May 6, 2022 •

edited

Loading

vnkozlov commented May 6, 2022 •

edited

Loading

navyxliu commented May 6, 2022 •

edited

Loading

navyxliu commented Jun 6, 2022 •

edited

Loading

openjdk bot commented Jun 13, 2022 •

edited

Loading