RFR: 8371716: C2: Phi node fails Value()'s verification when speculative types clash [v5]

[Marc Chevalier]

> This bug was originally found and reported as a Valhalla problem. It quickly became apparent it has no reason to be Valhalla-specific, while I couldn't prove so. Roland managed to make a mainline reproducer. The explanation details my Valhalla investigation, but it has nothing to do with value classes anyway.
> 
> The proposed solution seems somewhat controversial. See https://github.com/openjdk/valhalla/pull/1717 for some previous discussion. Before polishing the PR, I'd like to reach an agreement on the way to go.
> 
> # Analysis
> ## Obervationally
> ### IGVN
> During IGVN, in `PhiNode::Value`, a `PhiNode` has 2 inputs. Their types are:
> 
> in(1): java/lang/Object * (speculative=TestSpeculativeTypes$C2:NotNull:exact * (inline_depth=3))
> in(2): null
> 
> We compute the join (HS' meet):
> https://github.com/openjdk/jdk/blob/09b25cd0a24a4eaddce49917d958adc667ab5465/src/hotspot/share/opto/cfgnode.cpp#L1299-L1306
> 
> t=java/lang/Object * (speculative=compiler/igvn/ClashingSpeculativeTypePhiNode$C2:exact *)
> 
> But the current `_type` (of the `PhiNode` as a `TypeNode`) is
> 
> _type=java/lang/Object * (speculative=compiler/igvn/ClashingSpeculativeTypePhiNode$C1:exact *)
> 
> We filter `t` by `_type`
> https://github.com/openjdk/jdk/blob/09b25cd0a24a4eaddce49917d958adc667ab5465/src/hotspot/share/opto/cfgnode.cpp#L1321
> and we get
> 
> ft=java/lang/Object *
> 
> which is what we return. After the end of `Value`, the returned becomes the new `PhiNode`'s `_type`.
> https://github.com/openjdk/jdk/blob/09b25cd0a24a4eaddce49917d958adc667ab5465/src/hotspot/share/opto/phaseX.cpp#L2150-L2164
> and
> https://github.com/openjdk/jdk/blob/09b25cd0a24a4eaddce49917d958adc667ab5465/src/hotspot/share/opto/node.cpp#L1117-L1123
> 
> 
> ### Verification
> On verification, `in(1)`, `in(2)` have the same value, so does `t`. But this time
> 
> _type=java/lang/Object *
> 
> and so after filtering `t` by (new) `_type` and we get
> 
> ft=java/lang/Object * (speculative=compiler/igvn/ClashingSpeculativeTypePhiNode$C2:exact *)
> 
> which is retuned. Verification gets angry because the new `ft` is not the same as the previous one.
> 
> ## But why?!
> ### Details on type computation
> In short, we are doing
> 
> t = typeof(in(1)) / typeof(in(2))
> ft  = t /\ _type (* IGVN *)
> ft' = t /\ ft    (* Verification *)
> 
> and observing that `ft != ft'`. It seems our lattice doesn't ensure `(a /\ b) /\ b = a /\ b` which is problematic for this kind of verfication that will just "try again and see if something change".
> 
> To me, the surprising fact was that the intersection
> 
> java/lang/Object * (...

Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:

  review

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/28331/files
  - new: https://git.openjdk.org/jdk/pull/28331/files/7a092dac..1c28403a

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=28331&range=04
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=28331&range=03-04

  Stats: 10 lines in 2 files changed: 0 ins; 0 del; 10 mod
  Patch: https://git.openjdk.org/jdk/pull/28331.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/28331/head:pull/28331

PR: https://git.openjdk.org/jdk/pull/28331