RFR: 8238812: assert(false) failed: bad AD file

[Rahul Raghavan]

Found that the earlier PR bug analysis and fix details done is wrong.
Request help to review this revised fix proposal.

Now understood need to fix two types of issues related to the bug-

(i). Crash with only CmpI nodes case.
Sample test case -
public static void test2() {
  for (int i = 5; i > -20; i -= 5) {
    switch (i) {
      case 0:
      case 10:
      case 20:
      case 30:
      case 40:
      case 50:
      case 60:
      case 100:
        res = 42;
        break;
    }
  }
}

// This generates the following subgraph:
/*
// i: -20..0
if (i != 0) {
  // i: -20..-1
  if (i < 0) {   <- /*This is always true but not folded by C2*/
    // Fall through
  } else {
    ...
    CastII(i-1, 0..4) <- /*Replaced by TOP because i-1 range is -21..-1 but still considered reachable by C2 although it is dead code*/
    ...
  }
} else {
  StoreI   <- Due to this additional store on, IfNode::has_shared_region returns false and the fold compares optimization does not kick in
}
*/

Sample IR extracts -
 77  Phi  ===  399  25  389  [[ 658  664  389  80  662  660  659 ]]
 25  ConI  ===  0  [[ 80  77 ]]  #int:0
 80  CmpI  === _  77  25  [[ 81  89 ]]
 81  Bool  === _  80  [[ 82 ]] [ne]
 82  If  ===  399  81  [[ 85  87 ]]
 85  IfFalse  ===  82  [[ 102  336 ]]
 87  IfTrue  ===  82  [[ 90 ]] #1
 89  Bool  === _  80  [[ 90 ]] [lt]
 90  If  ===  87  89  [[ 91  92 ]]
 91  IfTrue  ===  90  [[ 102 ]] #1
 92  IfFalse  ===  90  [[ 156 ]] #0
 336  StoreI  ===  85  359  335  332  [[ 337 ]]
 156  Jump  ===  92  1  [[ 157  160  .......

Fix proposal here includes :
- Extract the existing non-CmpU related optimization out of 'fold_compares_helper()',
- form new method
(new 'fold_dominated_if()' added and renamed fold_compares_helper as 'fold_to_unsigned()').
- Call it properly from fold_compares() and related code adjustments.




(ii). 'fold_compares()' not handling CmpU nodes cases.

Sample basic test case -
public static void test3() {
  int local = 0;
  for (int i = 5; i > -20; i -= 5) {
    switch (i) {
    case 0:
    case 10:
    case 20:
    case 30:
    case 40:
    case 50:
    case 100:
      local = 42;
      break;
    }
  }
}

// This generates the following subgraph:
/*
// i: -20..0
if (i != 0) {
  // i: -20..-1
  if (i u< 101) {    <- /*This is always false but not folded by C2 because CmpU is not handled*/
    CastII(i-1, 0..49)  <- /*Replaced by TOP because i-1 range is -21..-1 but still considered reachable by C2 although it is dead code*/
  } else {
    ...
  }
} else {
  ...
}
*/

Sample IR extracts -
 77  Phi  ===  363  20  353  [[ 573  577  572  80  575  353  345  574 ]]
 20  ConI  ===  0  [[ 80  77 ]]  #int:0
 80  CmpI  === _  77  20  [[ 81 ]]
 81  Bool  === _  80  [[ 82 ]] [ne]
 82  If  ===  363  81  [[ 85  87 ]]
 85  IfFalse  ===  82  [[ 102 ]] #0
 87  IfTrue  ===  82  [[ 337 ]] #1
 95  ConI  ===  0  [[ 345 ]]  #int:101
 345  CmpU  === _  77  95  [[ 346 ]]
 346  Bool  === _  345  [[ 337 ]] [lt]
 337  If  ===  87  346  [[ 338  339 ]]
 338  IfFalse  ===  337  [[ 102 ]] #0
 339  IfTrue  ===  337  [[ 135 ]] #1
 135  Jump  ===  339  1  [[ 136  139  142  .....

Proposed fix includes:
Enabling Op_CmpU support in -
- 'cmpi_folds()',
- 'filtered_int_type()' - returns possible restrictive type for input value based on condition control flow for the if (supported CmpU for various condition control types).
- 'fold_dominated_if()' - checks if dominating if determines the result of current if and above example test case is a basic variant possibility where both CmpNodes compare a common value directly. (Node 77 Phi in above sample). But as mentioned in the comments of new helper function get_base_comparing_value(), there are other possibilities where there might be an AddINode with a constant increment present in between the CmpNodes and the common value compared. 
(Sample IR extract with AddI in between CmpNodes  and common value compared:  )
 76  Phi  ===  364  79  354  [[ 576  580  575  80  578  354  344  577 ]]  #int:0..20:www
 79  ConI  ===  0  [[ 80  76 ]]  #int:20
 80  CmpI  === _  76  79  [[ 81 ]]
 81  Bool  === _  80  [[ 82 ]] [ne] 
 82  If  ===  364  81  [[ 85  87 ]]
 85  IfFalse  ===  82  [[ 102 ]] #0
 87  IfTrue  ===  82  [[ 338 ]] #1
 343  ConI  ===  0  [[ 344 ]]  #int:-20
 344  AddI  === _  76  343  [[ 348 ]] 
 347  ConI  ===  0  [[ 348 ]]  #int:281
 348  CmpU  === _  344  347  [[ 349 ]] 
 349  Bool  === _  348  [[ 338 ]] [lt]
 338  If  ===  87  349  [[ 339  340 ]]
 339  IfFalse  ===  338  [[ 102 ]] #0 
 340  IfTrue  ===  338  [[ 136 ]] #1
 136  Jump  ===  340  1  [[ 137  140  143  146 .........
All these variants are handled in get_base_comparing_value() and related test cases added.

- and related changes in fold_compares().


Found no issues with this proposed fix for various unit tests, reported java_fuzzer case, tier1-6 and pre-integeration tests

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

Commit messages:
 - 8238812: assert(false) failed: bad AD file
 - 8238812: assert(false) failed: bad AD file
 - 8238812: assert(false) failed: bad AD file

Changes: https://git.openjdk.java.net/jdk/pull/2758/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2758&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8238812
  Stats: 393 lines in 3 files changed: 327 ins; 22 del; 44 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2758.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2758/head:pull/2758

PR: https://git.openjdk.java.net/jdk/pull/2758