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

[Xin Liu]

On Thu, 1 Jul 2021 19:23:19 GMT, Vladimir Kozlov <kvn 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 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.
>
>> > 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.

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

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