RFR: 8304948: [vectorapi] C2 crashes when expanding VectorBox [v2]

[Eric Liu]

On Sat, 22 Apr 2023 09:38:37 GMT, Eric Liu <eliu at openjdk.org> wrote:

>> I do still not fully grasp it though. When `expand_vbox_helper` is called on `Phi1`, it creates `NewPhi1`, then it calls `expand_vbox_helper` on `Phi2`, a `NewPhi2` is created, then `expand_vbox_helper` is invoked again on `Phi1`, which it shortcircuits to return `Phi1`, which will be attached as an input of `NewPhi2`, at this step should the second invocation on `Phi1` returns `NewPhi1` instead?
>> 
>> Thanks a lot.
>
> Yes, I think it should return the `NewPhi1` instead. 
> 
> In my test case, the `NewPhi1` and `NewPhi2` are idealized to `Phi1` and `Phi2`, so it does not matter whether it returns the new one. But I'm not sure if it's certain to be idealize to the old one. Anyway, return the new is more reasonable. I will fix that and do test.

> Hi @e1iu , Can you please elaborate the reason for CYCLIC Phi creation in this case. 

To explain this issue, I also added a test case with the IR graph. The cycled Phi, I think is normal in C2 especially when it's two tiered loop. 

The two Phi nodes represent the value of `a` after inner loop and outer loop respectively. After outer loop, `a` should be a Phi(Phi1), one value is `a0` which is the initial value, another is the value after inner loop. The value of `a` after inner loop should also be a Phi(Phi2), one value is the initial  `a1`, another is the value  after outer loop, which is  Phi1.

In Vector API, `a0` and `a1` can be VectorBox.

> Ideally we should not have seen that pallet in first place. 

Cycled Phi has already been taken into consideration in Phi optimization. Please refer to the links [2] and [3]. If we disabled this shape in merge_thorugh_phi, I think the bug will be gone in this place. But that would loss some chances matching VectorBox and VectorUnbox.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13489#discussion_r1176736542