[jdk17] RFR: 8269771: assert(tmp == _callprojs.fallthrough_catchproj) failed: allocation control projection [v2]

[Vladimir Kozlov]

On Fri, 2 Jul 2021 01:39:29 GMT, Xin Liu <xliu at openjdk.org> wrote:

>>> > I suggest to create new **open** bug and close current as dup.
>>> 
>>> This fixes the original reported bug - so that we run fine even without JDK-8268405 (which probably doesn't fix it - just hides it.)
>> 
>> I mean this bug 8269641 is marked as `Confidential`. Nobody can see it, even GitHub - see `Issue`. You need to create new bug which does not have our confidential information for review in open GitHub.
>> 
>>> 
>>> > I also don't get why your change in macro.cpp fixed the issue. `fallthrough_catchproj` is local cached value - it should not be updated with call to `replace_node()`.
>>> > Can you tell more what happened here?
>>> 
>>> fallthrough_catchproj is chached. It's the tmp that will be NULL if the assert is after the replace. The Init-node is being removed. Maybe I should just remove the entire assert - it's a bit unnecessary.
>> 
>> It looks like `InitalizeNode` has only control projection output and when it is replaced it also removes `InitalizeNode` as dead node too. In such case you just need to relax assert by checking `tmp == NULL ||` and adding comment that `InitalizeNode` can become dead when it has only one projection.
>
> hi, @vnkozlov , 
> 
> Are we handling code like this? 
> 
>  \
> -o----------
> | Initializer |
> -----------
>   \
>    o------
>    | Halt   |  
>    ---------
> 
> 
>> It looks like InitalizeNode has only control projection output and when it is replaced it also removes InitalizeNode as dead node too. 
> 
> Yes, ` replace_node(ctrl_proj, init->in(TypeFunc::Control));` calls `subsume_node(halt, init->in(TypeFunc::Control))`, and it will empty halt's outputs, but can we say that init->in(TypeFunc::Control) is NULL after replace_node()?  
> Initializer is a dead node after then, but I don't see its in(0) become NULL.

hi @navyxliu
`ctrl_proj` and `init->in(TypeFunc::Control)` (which is Allocation's control out `Proj` node) are separate `Proj` nodes. After calling  `replace_node()` `Halt` node will be attached to Allocation's control out `Proj` node.
 No users of `ctrl_proj` left after control edge of Halt node is replaced. `remove_dead_node(old)` is called for it: [phaseX.cpp#L1488](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/phaseX.cpp#L1488)
It calls `remove_globally_dead_node(dead)` which replaces input edges of `ctrl_proj` with NULL:
[phaseX.cpp#L1379](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/phaseX.cpp#L1379) 
After that it checks if its input (which is `Initalize` node) has any other outputs (users). If it does not have any - it is also dead and placed on stack for removal. As result `Initalize` node have its **all** inputs edges replaced with NULL too.

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

PR: https://git.openjdk.java.net/jdk17/pull/193