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

[Doerr, Martin]

Hi Patric,

excellent. I have found issues with the surgery while running some tests.
The VM was running into assert(cnt == _outcnt) failed: no insertions allowed
due to usage of DUIterator_Last.
So a version without the surgery sounds promising.

Best regards,
Martin


> -----Original Message-----
> From: hotspot-compiler-dev <hotspot-compiler-dev-
> bounces at openjdk.java.net> On Behalf Of Patric Hedlin
> Sent: Freitag, 15. November 2019 10:28
> To: Vladimir Ivanov <vladimir.x.ivanov at oracle.com>; hotspot compiler
> <hotspot-compiler-dev at openjdk.java.net>
> Subject: Re: RFR(M): 8220376: C2: Int >0 not recognized as !=0 for div by 0
> check
> 
> Thanks Vladimir for pointing this out.
> 
> The immediate reason is that this is a reduced patch derived from a more
> general one (addressing a few more cases and requiring some "surgery").
> I thought I should put this out first (to address the very limited case
> in the trouble report) as I found the more general approach to grow a
> bit more than I was happy with (the intent being to follow-up with the
> more general version later). But you are absolutely right that this
> single case only requires a constant condition. I'll re-work the patch
> and save the "surgery" for later.
> 
> Best regards,
> Patric Hedlin
> 
> On 14/11/2019 09:31, Vladimir Ivanov wrote:
> >
> >
> >> Webrev: http://cr.openjdk.java.net/~phedlin/tr8220376/
> >
> > I just briefly looked through the patch and have a quick high-level
> > question:
> >
> > +  // Rewrite:
> > +  //              cmp                   cmp
> > +  //              / \                    |
> > +  //     (r1)  bool  \                  bool (r1)
> > +  //            /    bool (r2)            \
> > +  //    (dom) if       \        ==>       if
> > +  //            \       )                  \
> > +  //    (pre)  if[TF]  /                  if[TF]X
> > +  //               \  /
> > +  //                if (this)
> > +  //               /  \
> > +  //             ifT  ifF [X]
> >
> > Why do you do complex graph surgery instead of simply adjusting
> > condition at redundant If (to 0/1) and let existing logic to eliminate
> > it?
> >
> > Best regards,
> > Vladimir Ivanov
> >
> >>
> >> 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
> >>