RFR (S) 8141044: C1 should fold (this == null) to false

[Vitaly Davidovich]

Aleksey,

Doesn't your patch only work for the implicit 'this' argument to instance
methods? It seems like only local at slot 0 is marked as receiver - what if
'this' is passed as one of the remaining args?

sent from my phone
On Nov 1, 2015 7:59 AM, "Aleksey Shipilev" <aleksey.shipilev at oracle.com>
wrote:

> Hi John,
>
> On 11/01/2015 05:10 AM, John Rose wrote:
> > On Oct 31, 2015, at 5:52 PM, Aleksey Shipilev
> > <aleksey.shipilev at oracle.com> wrote:
> >>
> >> Hi,
> >>
> >> I would like to suggest a simple improvement in C1: folding "(this
> >> == null)" to "false". This is a minor nit in A*FU performance with
> >> C1.
> >>
> >> JIRA: https://bugs.openjdk.java.net/browse/JDK-8141044
> >>
> >> Webrev: http://cr.openjdk.java.net/~shade/8141044/webrev.01/
> >>
> >> The patch passes JPRT, and improves the generated code in targeted
> >> benchmarks.
> >
> > What happens if javac uses Local[0] for another value, and that value
> > is null?
>
> Um. I think there is a confusion between "slots" in bytecode and Locals
> in C1. My cursory reading of C1 code tells me that only the receiver and
> arguments are exposed as Local-s in C1: see the patch, where only
> GraphBuilder::state_at_entry creates Locals, and also it is said:
>
> // A local is a placeholder for an incoming argument to a function call.
> LEAF(Local, Instruction)
>
> It would seem that we track what instruction had born the value, and
> Local is a placeholder for "look, it was coming from the outside".
> Therefore, I think a subsequent astore_0/aload_0 cannot be confused as
> the receiver.
>
> > Something has to clear out the _is_receiver bit if Local[0] can ever
> > be set to non-null.
>
> That's the thing: I cannot see how it could be set to non-null... If we
> ever overwrite the slot, we overwrite the Local[0] completely, including
> its _is_receiver, right?
>
> > Here's a bad case of how it can go wrong:
>
> > foo(x, y) {
> > entry:
> >   Local[0] = x; Local[0].is_recv = true;
> >   Local[1] = y;
> > loopHead:
> >   for (;;) {
> >     if (Local[0] == null)  doSomethingValid();  // SHOULD reach here, if
> y==null
> >   trickyBit:
> >     Local[0] = Local[1];
> >   }
> > }
>
> I don't understand this example, sorry. What does it mean, "Local[0] =
> Local[1]"? The example seems to assume that Local is some kind of
> indexed table where we store values. But in C1, Local is just a Value
> box which stores the incoming function argument.
>
> Thanks,
> -Aleksey
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20151101/453ca8cb/attachment.html>