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