RFR: 8252583: Make PhiNode::is_copy() debug only [v2]

[Roberto Castañeda Lozano]

On Tue, 22 Sep 2020 05:04:56 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> Sorry, I am not sure this is better than original code where assert is in one place - in PhiNode::is_copy().
>> The method is in .hpp file and is inlined - NULL check will be eliminated. It will only be executed in slowdebug build.
>> May be we should "bite the bullet" and remove this method at all - we don't hit the assert in years.
>> We can replace is_copy() check with assert in PhiNode::Ideal() - that should be enough to guarantee that we don't
>> create Phi with NULL control edge.
>
>> Sorry, I am not sure this is better than original code where assert is in one place - in PhiNode::is_copy().
>> The method is in .hpp file and is inlined - NULL check will be eliminated. It will only be executed in slowdebug build.
>> May be we should "bite the bullet" and remove this method at all - we don't hit the assert in years.
>> We can replace is_copy() check with assert in PhiNode::Ideal() - that should be enough to guarantee that we don't
>> create Phi with NULL control edge.
> 
> Another thing is that  a node has a member called uint Node::is_Copy() const.  PhiNode/Region nodes both have a member
> callled "is_copy()".  Is it intentional?  IMHO, it's not good in style.

Thanks Tobias and Xin for the reviews! I updated the PR for the record, even if we might end up moving the assertions
somewhere else as per Vladimir's suggestion.

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

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