[14] RFR(S): 8233656: assert(d->is_CFG() && n->is_CFG()) failed: must have CFG nodes

[Tobias Hartmann]

On 12.11.19 10:39, Vladimir Ivanov wrote:
> (Assuming in(0) == NULL or TOP or Proj(_, TOP) are the only cases we need to care about.)
> 
> Currently, ConstraintCastNode::dominating_cast() does NULL check, PhaseGVN::is_dominator_helper()
> does TOP check, but Proj(TOP) is left uncovered and it reaches d->is_CFG() check.
> 
> Your fix covers both TOP and Proj(_, TOP) on ConstraintCastNode::dominating_cast() by doing is_CFG()
> check.
> 
> What I'm in favor of is to handle Proj(TOP) case explicitly and there are other places in the code
> base which do that. (It may sound too subtle, but it doesn't look right when the code performs
> in(0)->is_CFG() check outside of an assert.)
> 
> I mentioned InitializeNode::detect_init_independence() as an example how control info processing can
> be done [1]. It covers NULL, TOP, and Proj(TOP) cases, but without is_CFG() check.
> 
> Considering current shape of ConstraintCastNode::dominating_cast() and that
> PhaseGVN::is_dominator_helper() already assumes non-NULL inputs, putting control normalization
> before TOP checks should solve the problem as well:
> 
>   bool PhaseGVN::is_dominator_helper(Node *d, Node *n, bool linear_only) {
>   + if (d->is_Proj())  d = d->in(0);
>   + if (n->is_Proj())  n = n->in(0);
>     if (d->is_top() || n->is_top()) {
>       return false;
>     }

Okay, let's go with that version then:
http://cr.openjdk.java.net/~thartmann/8233656/webrev.01/

I've verified that it still fixes the problem.

Thanks,
Tobias