RFR: 8275202: C2: optimize out more redundant conditions

[Roland Westrelin]

On Fri, 6 Oct 2023 01:33:31 GMT, Dean Long <dlong at openjdk.org> wrote:

> I'm probably missing something, but isn't there a simpler way to accomplish this? If we create a new node, then we can attach the constrained type to that node. So instead of relying on dominator information for the constrained type, we just grab the current value of the local:
> 
> ```
>   if (i < 10) {
>     i = ConstraintCast("i < 10", i); // injected
>     if (i < 42) {  // sees new constrained type above
>     }
>   } else {
>     i = ConstraintCast("i >= 10", i);
>     if (i < 42) {
>     }
>   }
> ```
> 
> In case of nested ifs, we end of up with ConstraintCast(ConstraintCast(...)) to flatten appropriately.

That's another way to achieve the same result but I don't think it's simpler:

- there would be a lot more casts in the graph. So pattern matching that c2 relies on almost everywhere including `Ideal` or `Identity` calls would have to be updated to take the extra cast nodes into account. That would maybe be ok if we had tests for every transformation in the compiler. We don't but making extensive use of casts would essentially require writing them.
- casts are a source of bugs. A change to `PhaseIdealLoop::try_sink_out_of_loop()` caused more casts to be added to pin nodes out of loops. They exposed inconsistencies in types between the control and data subgraphs and we've been struggling to fix all bugs that the fuzzer has found since.
- the way casts are added at parse time doesn't cover all cases. So:

if (field >= 0) {
  if (field <= 10) {
    // use field

Cast are propagated to uses by replacing a local's value with a cast in the c2 parse time map. If `field` is loaded from memory twice, then replacing the local for the first load with a narrowed type doesn't propagate tho the second load.
- casts introduce extra dependencies. So for instance:

if (i >= 0) {
  v = array[i];
  if (i <= 10) {
    v += array[i];

The 2 loads should common but if we add cast nodes after each if, then the address computed for each array load is different because it depends on a different cast node and the loads won't common.

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

PR Comment: https://git.openjdk.org/jdk/pull/14586#issuecomment-1752526416