RFR: 8263361: Incorrect arraycopy stub selected by C2 for SATB collectors

[Nils Eliasson]

On Mon, 15 Mar 2021 13:39:29 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> This bug has only been reproduced with -XX:+StressReflectiveCode and -XX:-ReduceInitialCardMarks. If StressReflectiveCode is necessary isn't obvious.
>> 
>> I am fixing three things:
>> 
>> 1) When running with ReduceInitialCardMarks enabled the stubroutines for int- or long-arraycopy will be used for arraycopies to tightly allocated arrays (like when cloning). With ReduceInitialCardMarks disabled the oop-version will be used with barriers.
>> 
>> The generate_arraycopy method in PhaseMacroExpand have a bool dest_uninitialized field - but that field denotes that we have proven that no zeroing is necessary (since all fields will be written anyway). 
>> 
>> To fix this bug generate_unchecked_arraycopy with the param dest_uninitialized will need to be appended with a check for tighly coupled object-copies when ReduceInitialCardMarks is disabled.
>> 
>> 2) I also fix a bug in library_call kit where arraycopy for clones doesn't get marked as tightly coupled - The call to tightly_coupled_allocation can't be used when the IR isn't constructed yet.
>> 
>> 3) In stubroutines.cpp I fix so that the name of the arraycopy stubroutines gets appended with _uninit when appropriate. This shows up in ir-dumps and IGV.
>> 
>> Please review,
>> Nils Eliasson
>
> src/hotspot/share/opto/library_call.cpp line 4147:
> 
>> 4145:           // Generate a direct call to the right arraycopy function(s).
>> 4146:           // Clones are always tightly coupled.
>> 4147:           ArrayCopyNode* ac = ArrayCopyNode::make(this, true, obj, intcon(0), alloc_obj, intcon(0), obj_length, true, false);
> 
>> I also fix a bug in library_call kit where arraycopy for clones doesn't get marked as tightly coupled - The call to tightly_coupled_allocation can't be used when the IR isn't constructed yet.
> 
> Can you elaborate, please, on the problem with `tightly_coupled_allocation()`? 
> If it can't be relied upon during parsing, then all other usages in `LibraryCallKit` have the same problem, don't they?

Yes - they might all be wrong - but it depends. In this particular case - tightly_coupled_allocation tries to look at the false path too see if it is a uncommon_trap - which is the only allowed control flow. But that part hasn't been expanded yet - so it is only a safepoint holding the parsing state.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3008