RFR(M): 8220376: C2: Int >0 not recognized as !=0 for div by 0 check

[Vladimir Ivanov]

> Webrev: http://cr.openjdk.java.net/~phedlin/tr8220376/

Looks good, Patric.

Best regards,
Vladimir Ivanov

PS: some cleanup suggestions (feel free to ignore them if you don't agree):

src/hotspot/share/opto/ifnode.cpp:

+//   \r1
+//  r2\  eqT  eqF  neT  neF  ltT  ltF  leT  leF  gtT  gtF  geT  geF
+//  eq    t    f    f    t    f    -    -    f    f    -    -    f
+//  ne    f    t    t    f    t    -    -    t    t    -    -    t
+//  lt    f    -    -    f    t    f    -    f    f    -    f    t
+//  le    t    -    -    t    t    -    t    f    f    t    -    t
+//  gt    f    -    -    f    f    -    f    t    t    f    -    f
+//  ge    t    -    -    t    f    t    -    t    t    -    t    f
+//
+Node* IfNode::simple_subsuming(PhaseIterGVN* igvn) {
+  // Table encoding: N/A (na), True-branch (tb), False-branch (fb).
+  static enum { na, tb, fb } s_subsume_map[6][12] = {
+  /*rel: eq+T eq+F ne+T ne+F lt+T lt+F le+T le+F gt+T gt+F ge+T ge+F*/
+  /*eq*/{ tb,  fb,  fb,  tb,  fb,  na,  na,  fb,  fb,  na,  na,  fb },
+  /*ne*/{ fb,  tb,  tb,  fb,  tb,  na,  na,  tb,  tb,  na,  na,  tb },
+  /*lt*/{ fb,  na,  na,  fb,  tb,  fb,  na,  fb,  fb,  na,  fb,  tb },
+  /*le*/{ tb,  na,  na,  tb,  tb,  na,  tb,  fb,  fb,  tb,  na,  tb },
+  /*gt*/{ fb,  na,  na,  fb,  fb,  na,  fb,  tb,  tb,  fb,  na,  fb },
+  /*ge*/{ tb,  na,  na,  tb,  fb,  tb,  na,  tb,  tb,  na,  tb,  fb }};

IMO you can dump the table from the comment: it mostly duplicates the 
code. (Probably, you can use a different name for "N/A" or just refer to 
it in numeric form (0?) to preserve clean structure of the table from 
the comment.)

======================================

+  if (is_If() && (cmp = in(1)->in(1))->Opcode() == Op_CmpP) {
+    if (cmp->in(2) != NULL && // make sure cmp is not already dead
+        cmp->in(2)->bottom_type() == TypePtr::NULL_PTR) {

Merge nested ifs?

======================================

Looks like extracting the following code into a helper function (along 
with the enum and the table) can improve readability.

+  int drel = subsuming_bool_test_encode(dom->in(1));
+  int trel = subsuming_bool_test_encode(bol);
+  int bout = pre->is_IfFalse() ? 1 : 0;
+
+  if (drel < 0 || trel < 0) {
+    return NULL;
+  }
+  int br = s_subsume_map[trel][2*drel+bout];
+  if (br == na) {
+    return NULL;
+  }

New function can return intcon(0/1) or bol(or NULL?) and the caller 
decides whether the update is needed.

> On 12/11/2019 15:16, Patric Hedlin wrote:
>> Dear all,
>>
>> I would like to ask for help to review the following change/update:
>>
>> Issue:  https://bugs.openjdk.java.net/browse/JDK-8220376
>> Webrev: http://cr.openjdk.java.net/~phedlin/tr8220376/
>>
>> 8220376: C2: Int >0 not recognized as !=0 for div by 0 check
>>
>>     Adding a simple subsumption test to IfNode::Ideal to enable a local
>>     short-circuit for (obviously) redundant if-nodes.
>>
>> Testing: hs-tier1-4, hs-precheckin-comp
>>
>>
>> Best regards,
>> Patric
>>
>