RFR: 8280541: remove self-recursion of ConnectionGraph::find_inst_mem() [v2]

Dean Long dlong at openjdk.java.net
Tue Feb 8 01:30:06 UTC 2022


On Wed, 2 Feb 2022 07:12:47 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> This is a follow-up task of JDK-8276219.
>> 
>> ConnectionGraph::find_inst_mem() contains a self-recursion for MergeMemNode.
>> It drills down into one input of MergeMemNode and tries to locate the memory node
>> which has the exact alias_idx. Once it returns, the result won't change from
>> recursion. Therefore, it's not necessary to use recursion in this case. We can
>> reset the initial state of this function and respin.
>> 
>> We can use a collection to remember all MergeMem Nodes and update them after then. 
>> 
>> This patch also makes a cleanup in MergeMemNode::memory_at(). C is not in use in
>> that function.
>
> Xin Liu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add an assertion in debug build.
>   
>   This patch ensures the result is same as recursion.

If the result won't change, then perhaps the self-recursion case should return instead of falling through.  Then it really is tail-recursion elimination.  However, knowing that falling through won't change the value makes several assumptions based on the current code, and those assumptions could break if the code changes.  To follow the original algorithm, the new version should fall through, which means having the loop pop elements off a stack.  Either way, this is not a trivial change to me.

I also noticed that moving the C->failing() check could also cause different behavior, but maybe it's harmless.

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

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


More information about the hotspot-compiler-dev mailing list