RFR: 8262739: String inflation C2 intrinsic prevents insertion of anti-dependencies [v2]

[Christian Hagedorn]

On Tue, 16 Mar 2021 12:57:38 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> The string inflation intrinsic `StrInflatedCopyNode` which converts the String internal byte[] to char[] has `bottom_type` `Type::MEMORY`:
>> https://github.com/openjdk/jdk/blob/f71e8a61988cc9f095a4f034636baf04c3f48f77/src/hotspot/share/opto/intrinsicnode.hpp#L143-L145
>> 
>> Memory users (i.e. loads) should therefore be checked for nearby anti-dependent stores:
>> https://github.com/openjdk/jdk/blob/f71e8a61988cc9f095a4f034636baf04c3f48f77/src/hotspot/share/opto/node.cpp#L1482-L1486
>> 
>> However, since `InstructForm::captures_bottom_type` returns `false` for `StrInflatedCopyNode`, after matching, the corresponding `MachNode` version will have `bottom_type` `Type::BOTTOM` and no anti-dependency analysis will be performed for its users. Scheduling then incorrectly orders loads from the char array with anti-dependent stores to the same array, leading to incorrect results.
>> 
>> `StrCompressedCopyNode` does not need this because it has `TypeInt::INT` as  `bottom_type` (the memory output is handled via a `SCMemProjNode`):
>> https://github.com/openjdk/jdk/blob/f71e8a61988cc9f095a4f034636baf04c3f48f77/src/hotspot/share/opto/intrinsicnode.hpp#L131-L133
>> 
>> Thanks,
>> Tobias
>
> Tobias Hartmann has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Moved check out of ifdef

Otherwise, the fix looks good!

Marked as reviewed by chagedorn (Reviewer).

src/hotspot/share/adlc/formssel.cpp line 781:

> 779:        !strcmp(_matrule->_rChild->_opType,"RotateRight")   ||
> 780: #if INCLUDE_SHENANDOAHGC
> 781:        !strcmp(_matrule->_rChild->_opType,"StrInflatedCopy") ||

Should not be inside `#if INCLUDE_SHENANDOAHGC`.

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

Changes requested by chagedorn (Reviewer).

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