RFR: 8307619: C2 failed: Not monotonic (AndI CastII LShiftI) in TestShiftCastAndNotification.java [v2]

[Emanuel Peter]

> **The Problem**
> 
> During CCP, we get to a state like that:
> 
> x (int:1)   Phi (int:4)
> |           |
> |     +-----+
> |     |
> LShiftI (int:16)
>   |
> CastII (top)       ConI (int:3)
>   |                 |
>   +----+  +---------+
>        |  |
>        AndI
> 
> 
> We call `AddINode::Value` during CCP, and in `MulNode::AndIL_shift_and_mask_is_always_zero` we `uncast` both inputs, which leaves us with `LShiftI` and `ConI` as the "true" inputs. They both have non-top types, and so we determine that this `AndI-LShiftI` combination always leads to `zero`: The `Phi` has a constant type (`int:4`). So this leaves the lowest 4 bits zero after the `LShiftI`. Then and-ing that with `int:3` means we extract the lowest 3 bits that are zero. So the result is provably always zero - that is the idea.
> 
> Then, we have some type updates (here of `x` and `Phi` and `LShiftI`), and the graph looks like this:
> 
> x (int)    Phi (int:0..4)
> |           |
> |     +-----+
> |     |
> LShiftI (int)
>   |
> CastII (top)       ConI (int:3)
>   |                 |
>   +----+  +---------+
>        |  |
>        AndI
> 
> 
> This leads to `shift2` failing to have constant type:
> https://github.com/openjdk/jdk/blob/a6b72f56f56b4f33ac163e90b115d79b2b844999/src/hotspot/share/opto/mulnode.cpp#L1964-L1967
> 
> And with that, we fall back to `MulNode::Value`:
> https://github.com/openjdk/jdk/blob/a6b72f56f56b4f33ac163e90b115d79b2b844999/src/hotspot/share/opto/mulnode.cpp#L559-L566
> 
> In `MulNode::Value` we detect that the `CastII` has type `top`, and return `top` for `AndI`.
> 
> CCP expects the types to become more wide over time, so going from `int:0` to `top` is the wrong direction.
> 
> **Solution**
> 
> The problem is with the relatively rare `CastII` still being `top` - this seems to be very rare. But the new regression test `TestShiftCastAndNotification.java` seems to create exactly that case, in combination with `-XX:StressCCP`.
> 
> We should guard against `top` in one of the `AndI` inputs inside `MulNode::AndIL_shift_and_mask_is_always_zero`. This will prevent it from detecting the zero-case, untill `MulNode::Value` would get a chance to compute a non-top type.
> 
> **Argument for Solution**
> 
> Is there still a threat from `MulNode::AndIL_shift_and_mask_is_always_zero` computing a zero first, and `MulNode::Value` a type that does not include zero after ward?
> As types only widen during CCP, having a zero first means that all inputs now are non-top - in fact they are all `T_INT`. Since types only widen in the input...

Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:

  refactor with @chhagedorn's suggestions

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/13908/files
  - new: https://git.openjdk.org/jdk/pull/13908/files/cf17d4df..4963faea

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

  Stats: 8 lines in 1 file changed: 3 ins; 3 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/13908.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/13908/head:pull/13908

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