JDK-8276219 : C2: ConnectionGraph::find_inst_mem() should not be recursive
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 18
  • Priority: P4
  • Status: Closed
  • Resolution: Won't Fix
  • Submitted: 2021-11-01
  • Updated: 2024-02-09
  • Resolved: 2022-02-01
Related Reports
Relates :  
JDK-8280541 - remove self-recursion of ConnectionGraph::find_inst_mem()
Relates :  
JDK-8276157 - C2: Compiler stack overflow during escape analysis on Linux x86_32
Relates :  
JDK-8324345 - Stack overflow during C2 compilation when splitting memory phi
Description
In JDK-8276157, Vladimir says:

"To actually fix the issue we would need to re-write recursive method ConnectionGraph::find_inst_mem() to normal method using Node_Stack or other C2's structures without recursion. Please, file RFE. May be also add check that it is not infinite recursion."

This would fix some stack overflows during escape analysis.
Comments
This change would reduce readability a lot. We need more justification before merging two functions into one.
01-02-2022
Yes, that's fine.
28-01-2022
is that okay I close this as "WONT FIX"? I can't come up an idea to solving case2 but still keep readability. We will revisit it if it does hurt performance in the future.
27-01-2022
Right, I think as long as we are fairly confident that we don't run out of stack and crash (like observed in JDK-8276157), readability/maintainability is more important than performance here.
24-01-2022
@thartmann, @shade Solving case2 is debatable. We have to incorporate split_memory_phi() into find_inst_mem(). That would reduce readability a lot. What's the tradeoff? Iteration is supposed to be faster than recursion, but EA isn't the performance bottleneck of C2, is it? case1 is the low hanging fruit. we can solve it first.
23-01-2022
There are two recursion cases in ConnectionGraph::find_inst_mem(). 1. self-recursion: when it encounters a MergeMem Node and memory_at(alias_idx) is base_memory(). 2. mutual recursion: split_memory_phi() invokes find_inst_mem() for its inputs. The case1 is easy to eliminate. The return value is all the same from recursion because it won't change. However, it seems that the stack overflow of JDK-8276157 is attributed to case2. The evidence is that I still see 900+ frames even I have solved case1. see the stacktraces as attachment. $make run-test TEST="java/lang/invoke/VarHandles/VarHandleTestAccessShort.java" JTREG="VM_OPTIONS=-XX:CompilerThreadStackSize=256" CONF=linux-x86_64-server-fastdebug
23-01-2022
Added the "starter" label but this might turn out to be difficult.
03-11-2021