AW: Node::find(int) should not traverse from new to old nodes

[Emanuel Peter]

Hi Vladimir and Tobias,

Thanks for the feedback.

Exactly, it is helpful to search old and new nodes, but we want better control - something more consistent.

You can find my suggested implementation here:
https://github.com/openjdk/jdk/pull/8567

I am still very open to suggestions, reviews and feedback :)

Thanks,
Emanuel
________________________________
Von: Tobias Hartmann <tobias.hartmann at oracle.com>
Gesendet: Freitag, 6. Mai 2022 08:25
An: Vladimir Kozlov <vladimir.kozlov at oracle.com>; Emanuel Peter <emanuel.peter at oracle.com>; hotspot-compiler-dev at openjdk.java.net <hotspot-compiler-dev at openjdk.java.net>
Betreff: Re: Node::find(int) should not traverse from new to old nodes

Thanks for the write-up. I agree with Vladimir, let's go with two separate find methods.

Best regards,
Tobias

On 05.05.22 18:16, vladimir.kozlov at oracle.com wrote:
> I think the original intention to search in old IR is to find any node with specified index. But you
> are right about inconsistency of implementation.
>
> I prefer to have a separate search of old nodes in separate find_old_node() method and remove such
> search from default find_node() (your Solution 1). It is still useful to look through old IR when
> debug.
>
> Thanks,
> Vladimir K
>
> On 5/5/22 2:25 AM, Emanuel Peter wrote:
>> Hi,
>>
>> I have been bothered by find_node(idx) for a while. When I am looking at the Mach graph, and
>> search for a node with an idx, I sometimes get old, sometimes new nodes. The reason is that
>> Node::find does not just traverse input/output edges, but also debug_orig (if ASSERT is enabled).
>> Via debug_orig, most Mach nodes link to their old node from the IR of previous phases. This way we
>> can find multiple nodes for an idx. Node::find returns the last one it finds - sometimes the new
>> one is last, sometimes the old one is last. At least it prints both pointers to the terminal.
>>
>> Here, I have a detailed writeup, and some proposed solutions:
>> https://bugs.openjdk.java.net/browse/JDK-8286179
>>
>> Do you agree that we should fix this? Would you pick one of my solutions, or propose a new one?
>> Since this is a tool that probably many people are using for debugging, I do not want to break it
>> for you.
>>
>> Best Regards,
>> Emanuel Peter