RFR: 8298848: C2: clone all of (CmpP (LoadKlass (AddP down at split if [v3]

[Roland Westrelin]

> As suggested by Vladimir in:
> https://github.com/openjdk/jdk/pull/11666
> 
> Thus extract one for the fixes as a separate PR. The bug as described
> in the above PR is:
> 
> The crash occurs because a` (If (Bool (CmpP (LoadKlass ..))))`
> only has a single projection. It lost the other projection because of
> a `CheckCastPP` that becomes `top`. Initially the pattern is, in pseudo
> code:
> 
> 
> if (obj.klass == some_class) {
>   obj = CheckCastPP#1(obj);
> }
> 
> 
> `obj` itself is a `CheckCastPP` that's pinned at a dominating if. That
> dominating if goes through split through phi. The `LoadKlass` for the
> pseudo code above also has control set to the dominating if being
> transformed. This result in:
> 
> 
> if (phi1 == some_class) {
>   obj = CheckCastPP#1(phi2);
> }
> 
> 
> with` phi1 = (Phi (LoadKlass obj) (LoadKlass obj))` and phi2 = (Phi obj obj)
> with `obj = (CheckCastPP#2 obj')`
> 
> `PhiNode::Ideal()` transforms `phi2` into a new `CheckCastPP`:
> `(CheckCastPP#3 obj' obj') `with control set to the region right above
> the if in the pseudo code above. There happens to be another
> `CheckCastPP` at the same control which casts obj' to a narrower
> type. So the new `CheckCastPP#3` is replaced by that one (because of
> `ConstraintCastNode::dominating_cast()`) and pseudo code becomes:
> 
> 
> if (phi1 == some_class) {
>   obj = CheckCastPP#1(CheckCastPP#4(obj'));
> }
> 
> 
> and then:
> 
> 
> if (phi1 == some_class) {
>   obj = top;
> }
> 
> 
> because the types of the 2 `CheckCastPP`s conflict. That would be ok if:
> 
> `phi1 == some_class`
> 
> would constant fold. It would if the test was:
> 
> `if (CheckCastPP#4(obj').klass == some_klass) {
> `
> but because of split if, the `(CmpP (LoadKlass ..))` and the
> `CheckCastPP#1` ended up with 2 different object inputs that then were
> transformed differently. The fix I propose is to have split if clone the entire:
> 
> `(Bool (CmpP (LoadKlass (AddP ..))))`
> 
> down the same way `(Bool (CmpP ..))` is cloned down. After split if, the
> pseudo code becomes:
> 
> 
> if (phi.klass == some_class) {
>   obj = CheckCastPP#1(phi);
> }
> 
> 
> The bug can't occur because the `CheckCastPP` and` (CmpP (LoadKlass ..))`
> operate on the same phi input. The change in split_if.cpp implements
> that.

Roland Westrelin has updated the pull request incrementally with five additional commits since the last revision:

 - Update src/hotspot/share/opto/split_if.cpp
   
   Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
 - Update test/hotspot/jtreg/compiler/types/TestCheckCastPPBecomesTOP.java
   
   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
 - Update src/hotspot/share/opto/split_if.cpp
   
   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
 - Update src/hotspot/share/opto/split_if.cpp
   
   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
 - Update src/hotspot/share/opto/split_if.cpp
   
   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/11689/files
  - new: https://git.openjdk.org/jdk/pull/11689/files/31fc40f7..e510a89b

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=11689&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11689&range=01-02

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

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