RFR: 8254269: simplify Node::disconnect_inputs [v2]

Xin Liu xliu at openjdk.java.net
Sun Oct 11 20:57:09 UTC 2020


On Sun, 11 Oct 2020 20:03:58 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Xin Liu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8254269: simplify Node::disconnect_inputs
>>   
>>   Optimize it for the precedence loop. because there's no null between
>>   2 non-null precedences, disconnect_inputs can break at a null value.
>
> src/hotspot/share/opto/node.cpp line 914:
> 
>> 912:   // Note: Safepoints may have precedence edges, even during parsing
>> 913:   // Precedences have no embedded NULL
>> 914:   for (; i < len() && in(i) != nullptr; ++i) {
> 
> I don't think it is correct.

hi, @cl4es  @vnkozlov,
Thank you to review this patch.

The reason I changed to this because I read this comment yesterday.
https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/node.hpp#L280

It suggests that `Node::_in` has an inherent order, which I depict here:
https://github.com/navyxliu/jdk/commit/b8a72755a29d2fcbe36c9dcaf7f696634f813b4f#diff-e1c5a771a82d5fdb7e88c5b90b444736R898

If C2 sees a nullptr in precedence section, it can assume the rest of precedences are all nullptr.
I have 2 evidence to prove it holds.
1. rm_prec() does relocate null value to the end.
https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/node.cpp#L1025 2. I added an assert after the
precedence loop and verified it using hotspot-tier1.   `for(; i<len(); ++i) assert(in(i) == nullptr, "non-null prec?");
`

With this assumption, c2 can leave loop a little bit early.

How about this? I withdraw this change and just make a pure clean-up.
I admit that the potential gain is very very small.  it's not worth it.

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

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


More information about the hotspot-compiler-dev mailing list