JDK-8371581 : C2: PhaseCCP should reach fixpoint by revisiting deeply-Value-d nodes
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 26
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2025-11-10
  • Updated: 2025-11-24
  • Resolved: 2025-11-24
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 26
26 masterFixed
Related Reports
Blocks :  
JDK-8360557 - CTW: Inline cold methods to reach more code
Relates :  
JDK-8298952 - All nodes should have type(n) == Value(n) after IGVN
Relates :  
JDK-8257197 - Add additional verification code to PhaseCCP
Description
Working on enabling JDK-8360557, there is a intermittent failure CTW-ing a 3rd party JAR:

# assert(!failure) failed: PhaseCCP not at fixpoint: analysis result may be unsound.
hs_err_com.ibm.icu.base-54.1.1.jar_0_10792.log

In the stderr, there is:

Missed Value optimization:
dist dump
---------------------------------------------
   1   222  AddP  === _ 68 68 221  [[ 230 463 ]]  !orig=[1460],[273],[394] !jvms: LocaleUtility::fallback @ bci:24 (line 121)
   1   403  StoreN  === 1490 230 1604 402  [[ 463 554 425 1277 479 455 457 460 ]]  @narrowoop: java/lang/Object *[int:>=0] (java/lang/Cloneable,java/io/Serializable)+any * [narrow], idx=6;  Memory: @narrowoop: java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):exact *[int:3] (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any * [narrow], idx=6; !orig=1590,[418] !jvms: LocaleUtility::fallback @ bci:44 (line 125)
   1   443  Proj  === 442  [[ 508 452 457 460 463 ]] #0 !jvms: LocaleUtility::fallback @ bci:61 (line 132)
   0   463  LoadN  === 443 403 222  [[ 464 ]]  @narrowoop: java/lang/Object *[int:>=0] (java/lang/Cloneable,java/io/Serializable)+any * [narrow], idx=6; #narrowoop: java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):exact * !jvms: LocaleUtility::fallback @ bci:73 (line 132)
Current type:
narrowoop: java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):exact *
Optimized type:
narrowoop: java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):exact *
Comments
Changeset: 99be0e73 Branch: master Author: Aleksey Shipilev <shade@openjdk.org> Date: 2025-11-24 07:47:13 +0000 URL: https://git.openjdk.org/jdk/commit/99be0e73ce9779e85c9ec6598e0a7ce964d62e82
24-11-2025
Seems this is more related with the CCP verification JDK-8257197 than IGVN verification.
14-11-2025
Hold on a second. With JDK-8346184, I see we have moved the block in LoadNode::Value that produced these bottom values for the block that "If we are loading from a freshly-allocated object, produce a zero, if the load is provably beyond the header of the object.". But in doing so, we have missed is_instance check?! Er. This resolves the CTW failure for me, but regresses JDK-8346184 unit test: ``` diff --git a/src/hotspot/share/opto/memnode.cpp b/src/hotspot/share/opto/memnode.cpp index f42a6ea9489..23e49e56bf0 100644 --- a/src/hotspot/share/opto/memnode.cpp +++ b/src/hotspot/share/opto/memnode.cpp @@ -2018,7 +2018,7 @@ const Type* LoadNode::Value(PhaseGVN* phase) const { const TypeOopPtr* tinst = tp->isa_oopptr(); bool is_instance = (tinst != nullptr) && tinst->is_known_instance_field(); Node* value = can_see_stored_value(mem, phase); - if (value != nullptr && value->is_Con()) { + if (value != nullptr && value->is_Con() && is_instance) { assert(value->bottom_type()->higher_equal(_type), "sanity"); return value->bottom_type(); } ``` I also do not understand why this block returns "bottom" (any value, afaics) when it wants to return a zero. Update: I suspect JDK-8346184 does it right by leaning heavily on can_see_stored_value() to produce the store that we can see, even if that store is not into instance field? That would make some sense; then the comment is just misleading: JDK-8371804.
13-11-2025
Ah yes, these have different base ptr types, TypeInstPtr::dump2 has a UX bug (JDK-8371789) that does not print it. After amending: Current type: narrowoop: java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):Constant:exact * 0x000071860c4a7868; hashcons: 0x000071860c4a7868 Optimized type: narrowoop: java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):BotPTR:exact * 0x00007186204c59c8; hashcons: 0x00007186204c59c8 Kinda weird that "optimized" switched from Constant to Bot, which I assume lost specificity.
13-11-2025
A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk/pull/28288 Date: 2025-11-13 10:49:14 +0000
13-11-2025
ILW = Assert in CCP due to not reaching fixpoint (potential wrong code being generated), reproducible with CTW and verification, no workaround but disable compilation of affected method = P3
13-11-2025
That looks reasonable to me. Thanks for working on this Aleksey!
13-11-2025
I think I am with [~qamai] here: it sounds like these "spooky action at a distance" nodes like Loads should be treated specially, instead of excepted in verfication. E.g. record these spooky nodes, check if their types are still refinable at the end of round, and if so, start next round, rinse and repeat until fully converged.
12-11-2025
> We are dealing with LoadN. The node *dump* already says the type is `BotPTR`. Yet Value verification says "current type" (which is one from CCP/GVN table) is still `Constant`, while "optimized type" (which is the `Value()` output for the node) is saying `BotPTR`. So I am thinking that the `Constant` -> `BotPTR` transform actually happened, yet CCP/GVN was not notified about it? I think is *is* what happens. The type of some node (Store?) was refined during CCP, but downstream Load-s are not notified by this change. So when LoadNode::Value goes into MemNode::can_see_stored_value and discovers that store far away, it refines its own type as well. But it is too late for CCP mainloop, and we only detect this during CCP verification. I YOLO-ed this experimental patch, and it goes farther, LoadN verification now passes, DecodeN verification passes... ``` diff --git a/src/hotspot/share/opto/phaseX.cpp b/src/hotspot/share/opto/phaseX.cpp index b5b15275e21..3b732be2d3f 100644 --- a/src/hotspot/share/opto/phaseX.cpp +++ b/src/hotspot/share/opto/phaseX.cpp @@ -2787,6 +2787,9 @@ void PhaseCCP::analyze() { Unique_Node_List worklist(&local_arena); DEBUG_ONLY(Unique_Node_List worklist_verify(&local_arena);) + Unique_Node_List loadn_wl(&local_arena); + Unique_Node_List decoden_wl(&local_arena); + // Push root onto worklist worklist.push(C->root()); @@ -2810,12 +2813,35 @@ void PhaseCCP::analyze() { set_type(n, new_type); push_child_nodes_to_worklist(worklist, n); } + if (n->is_Load()) { + loadn_wl.push(n); + } + if (n->is_DecodeN()) { + decoden_wl.push(n); + } if (KillPathsReachableByDeadTypeNode && n->is_Type() && new_type == Type::TOP) { // Keep track of Type nodes to kill CFG paths that use Type // nodes that become dead. _maybe_top_type_nodes.push(n); } } + // YOLO: Do final processing for nodes to capture their new types. + while (loadn_wl.size()) { + Node* n = loadn_wl.pop(); + const Type* new_type = n->Value(this); + if (new_type != type(n)) { + dump_type_and_node(n, new_type); + set_type(n, new_type); + } + } + while (decoden_wl.size()) { + Node* n = decoden_wl.pop(); + const Type* new_type = n->Value(this); + if (new_type != type(n)) { + dump_type_and_node(n, new_type); + set_type(n, new_type); + } + } DEBUG_ONLY(verify_analyze(worklist_verify);) } ``` Now we are stuck at CmpP verification, which looks much more concerning, as the range *widens*! ``` Missed Value optimization: dist dump --------------------------------------------- 1 71 ConP === 0 [[ 72 687 629 108 620 277 610 309 17 1212 1220 1451 1457 1514 1520 ]] # 3 0 0 2147483647 == null 1 464 DecodeN === _ 463 [[ 629 930 635 1007 508 ]] #java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):BotPTR:exact * Oop:java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):BotPTR:exact * !jvms: LocaleUtility::fallback @ bci:73 (line 132) 0 629 CmpP === _ 464 71 [[ 630 ]] !jvms: Locale::<init> @ bci:28 (line 812) LocaleUtility::fallback @ bci:74 (line 132) Current type: int:1 Optimized type: int:-1..1, 0u..maxuint ```
12-11-2025
I think we have another issue here, unlike GVN in which failure to converge simply means that we are not optimal enough, failure to converge during CCP means incorrect compilation. As a result, PhaseCCP::verify_analyze should not call PhaseIterGVN::verify_Value_for
12-11-2025
In PhaseIterGVN::verify_Value_for, there is an exception: // Exception (2) // LoadNode performs deep traversals. Load is not notified for changes far away. if (n->is_Load() && !told->singleton()) { // MemNode::can_see_stored_value looks up through many memory nodes, // which means we would need to notify modifications from far up in // the inputs all the way down to the LoadNode. We don't do that. return false; } ...which almost applies, but it is gated by `!told->singleton()`. If I remove that gating, and thus except all LoadNode-s, CTW starts to pass (obviously). But this one looks unsatisfactory, as it stops verifying all loads.
12-11-2025
> Does it make a difference if you return phase->type(value) instead? Nope. > I assume you can reproduce with JDK 25 as well but not before JDK-8346184, right? It is very fiddly to reproduce. I have lucked out on a single top commit in mainline with a specific seed where it reproduces reliably. So I am not even trying with JDK 25, because it is intermittent. Here is what I noticed today in the dump: ``` Missed Value optimization: dist dump --------------------------------------------- 1 222 AddP === _ 68 68 221 [[ 230 463 ]] !orig=[1460],[273],[394] !jvms: LocaleUtility::fallback @ bci:24 (line 121) 1 403 StoreN === 1490 230 1604 402 [[ 463 554 425 1277 479 455 457 460 ]] @narrowoop: java/lang/Object:BotPTR *[int:>=0] (java/lang/Cloneable,java/io/Serializable)+any * [narrow], idx=6; Memory: @narrowoop: java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):BotPTR:exact *[int:3] (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any * [narrow], idx=6; !orig=1590,[418] !jvms: LocaleUtility::fallback @ bci:44 (line 125) 1 443 Proj === 442 [[ 508 452 457 460 463 ]] #0 !jvms: LocaleUtility::fallback @ bci:61 (line 132) 0 463 LoadN === 443 403 222 [[ 464 ]] @narrowoop: java/lang/Object:BotPTR *[int:>=0] (java/lang/Cloneable,java/io/Serializable)+any * [narrow], idx=6; #narrowoop: java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):BotPTR:exact * !jvms: LocaleUtility::fallback @ bci:73 (line 132) Current type: narrowoop: java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):Constant:exact * Optimized type: narrowoop: java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):BotPTR:exact * ``` We are dealing with LoadN. The node *dump* already says the type is `BotPTR`. Yet Value verification says "current type" (which is one from CCP/GVN table) is still `Constant`, while "optimized type" (which is the `Value()` output for the node) is saying `BotPTR`. So I am thinking that the `Constant` -> `BotPTR` transform actually happened, yet CCP/GVN was not notified about it?
12-11-2025
I assume you can reproduce with JDK 25 as well but not before JDK-8346184, right?
12-11-2025
> I also do not understand why this block returns "bottom" But 'value' is a constant and the assumption is that the bottom_type of a ConNode (TypeNode) is a constant type, right? "bottom_type" just means the most conservative type but for a constant it's also the most precise type. Maybe that assumption does not hold here. Does it make a difference if you return phase->type(value) instead? Also, we are not "producing a zero" here, right? Or am I missing something? > can_see_stored_value() to produce the store that we can see It does return the stored value though, not the store. Could you print 'value' and 'this' in the failing case? I'm still not sure if I follow :)
12-11-2025
This seems to work well on both CTW reproducer I have, and also passing hotspot_compiler: https://github.com/openjdk/jdk/compare/master...shipilev:JDK-8371581-ccp-spooky-nodes (Includes JDK-8360557 for reproducibility.) I can polish and PR it, if we agree this is a sound approach.
12-11-2025
Honestly, this whole thing is confusing. It does not feel right that LoadNode::Value is able to move Constant to BotPTR type. But I cannot see how to prevent that without breaking JDK-8346184 tests. There is something I clearly do not understand in how current LoadNode Value/Identity/Ideal works.
11-11-2025
More debugging breadcrumbs, looks like the types are different even after applying hashcons over them. They are hashcons-ed to the same pointer, which likely means we are not missing hashcons anywhere. Current type: narrowoop: java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):exact * 0x00007f0ec5154448; hashcons: 0x00007f0ec5154448 Optimized type: narrowoop: java/lang/String (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/constant/Constable,java/lang/constant/ConstantDesc):exact * 0x00007f0e740b6978; hashcons: 0x00007f0e740b6978
11-11-2025
Yeah, I am trying to reproduce it, which would hopefully point me straight to the bug which I can fix :)
11-11-2025
I think '==' should be fine because 'Type::hashcons' should ensure that if two types are equivalent, they should have the same pointer. Maybe we forgot to call 'hashcons' somewhere or not all properties of the type are printed. [~shade] Could you provide some more details on how to reproduce this? Are you planning to work on the fix?
11-11-2025
I do wonder why JDK-8298952 checks types with `==`, and not with `equals`. In the dump above, the type dumps look identical. Aren't we supposed to have these types properly canonicalized (e.g. hashcon-ed)?
10-11-2025