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

[Vladimir Kozlov]

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).

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?

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

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"

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.
>