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

[Vladimir Ivanov]

Hi Tobias,

> Or do you mean checking ctl->in(0) for TOP?

Yes, I'm curious whether it's enough to check that in(0) is NULL, TOP, 
or Proj(_, TOP). Are there any other important cases left?

My concerns is that other usages of PhaseGVN::is_dominator() may be 
affected the same way as well.

It looks like PhaseGVN::is_dominator_helper() would benefit from 
additional checks:

bool PhaseGVN::is_dominator_helper(Node *d, Node *n, bool linear_only) {
   if (d->is_top() || n->is_top()) {
     return false;
   }
   assert(d->is_CFG() && n->is_CFG(), "must have CFG nodes");
   ...


As an example, InitializeNode::detect_init_independence() does some 
control normalization first:

bool InitializeNode::detect_init_independence(Node* value, PhaseGVN* 
phase) {
...
     if (n->is_Proj())   n = n->in(0);
...
     if (n->is_CFG() && phase->is_dominator(n, allocation())) {
       continue;
     }

     Node* ctl = n->in(0);
     if (ctl != NULL && !ctl->is_top()) {
       if (ctl->is_Proj())  ctl = ctl->in(0);
       if (ctl == this)  return false;
...
       if (!MemNode::all_controls_dominate(n, this))
...
bool MemNode::all_controls_dominate(Node* dom, Node* sub) {
   if (dom == NULL || dom->is_top() || sub == NULL || sub->is_top())
     return false; // Conservative answer for dead code

>> Also, is it worth putting an assert to ensure the node is already on worklist and will be eventually
>> eliminated?
> 
> I don't think it's worth it. Since 8040213 [1] we have code that ensures that all modified nodes are
> added to the worklist (see Compile::record_modified_node()). I've verified that the ProjNode with
> TOP input is covered by that.

Got it. Sounds good.

Best regards,
Vladimir Ivanov