RFR(S): 8146828: Subsequent arraycopy does not always eliminate array zeroing

[Roland Westrelin]

Thanks for taking a look at this, Vladimir.

> Yes, it is complicated code with assumptions not covered by comments.
> 
> Needs more comments in place where you check too_many_traps to explain why such reasons are checked (this bug description).

Here is a new webrev with more comments:
http://cr.openjdk.java.net/~roland/8146828/webrev.01/

> And it should be mentioned that during expansion of ArrayCopy node subtype and other checks do not generate uncommon traps (right?) so there will be no safepoints between moved allocation and arraycopy.
> 
> This change allow to move allocation when there were a lot Reason_null_check traps but not Reason_intrinsic. But in such case we generate explicit throw code for null checks (right). The could be catch path which assumes that allocation was finished. Right?

But in that case, the second call to tightly_coupled_allocation():

4760     alloc = tightly_coupled_allocation(dest, NULL);

would return null because something would observe the allocation.

> Null checks can use Reason_speculate_null_check reason (null_check_oop() call). Why we don't check for it?

I’ve added code to take care of that.

> Next comment is incomplete since you have 2 null checks, for src and dst and both should be optimized out:
> 
> 4729     // See if the null check above was optimized out (alloc not null)
> 
> The src check could be optimized if there is dominating check above allocation.
> 
> Typo: "we can’t emit any guards”

I fixed that as well.

Roland.

> 
> thanks,
> Vladimir
> 
> 
> On 2/11/16 6:41 AM, Roland Westrelin wrote:
>> http://cr.openjdk.java.net/~roland/8146828/webrev.00/
>> 
>> The problem is that we add a null check for src when we intrinsify arraycopy. So there's a null check between the allocation and the arraycopy itself and if we hit that null check we deoptimize with a live allocation and that allocation must be zeroed. We usually move the allocation below the predicates and arrange for the predicates in case they fail to resume execution in the interpreter right before the allocation. But in that case one of the predicate fails on a first compilation (subtype check) so we don't attempt it on the second one. In the change, the check for too many traps of type Reason_intrinsic and too many traps of type Reason_null_check are done independently so if we already hit too many Reason_intrinsic traps we still can move the null check above the allocation and eliminate zeroing. Hopefully I didn’t break anything in that very confusing code.
>> 
>> Roland.
>>