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

[Vladimir Kozlov]

On Sun, 11 Oct 2020 20:54:00 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> 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.

You are right. There is already existing assert in find_prec_edge() called from set_prec():
https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/node.hpp#L436

I still want to see explicit start from req():

for (uint i = req(); (i < len()) && (in(i) != nullptr); ++i) {
  set_prec(i, nullptr);
}

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

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