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

[Aleksey Shipilev]

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 --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20151101/775ecd97/signature.asc>