RFR: 8335221: Some C2 intrinsics incorrectly assume that type argument is compile-time constant [v2]
Vladimir Kozlov
kvn at openjdk.org
Thu Jun 27 23:31:18 UTC 2024
On Thu, 27 Jun 2024 23:25:28 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:
>> Problems were found in some C2 intrinsics while testing Leyden Early Release.
>>
>> [`inline_array_partition()`](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/library_call.cpp#L5493) and [`inline_array_sort()`](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/library_call.cpp#L5559) intrinsics incorrectly assume that `elementType` argument is always compile-time constant. Which is not true for Leyden (and in general) when constant folding optimization is switched off (or did not executed for particular type).
>>
>> Note, other intrinsics are very careful about that. For example, see [`inline_Class_cast()`](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/library_call.cpp#L3963) intrinsic.
>>
>> Add similar checks to `inline_array_partition()` and `inline_array_sort()`.
>>
>> An other problem is that `null_check()` (which adds new control nodes to graph) was executed in these intrinsics before bailouts from intrinsic generation. We hit next assert if bailout happens:
>>
>> assert(ctrl == kit.control(), "Control flow was added although the intrinsic bailed out");
>>
>>
>> I moved `null_check()` after bailouts.
>>
>> An other issue with `null_check()` is it checks for `null` first argument which is `Class`. It can't be `null` because corresponding Java methods are `private` and each call site passed `<primitive_type>.class` as first argument.
>> For example: [DualPivotQuicksort.java#L260](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/DualPivotQuicksort.java#L260)
>> On other hand there is no null check for passed array (second argument). I changed `null_check()` for array.
>>
>> These 2 intrinsics were added by [JDK-8309130](https://bugs.openjdk.org/browse/JDK-8309130) in JDK 22.
>>
>> Testing tier1-6, hs-stress, hs-xcomp
>
> Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision:
>
> Move common checks into shared method
Moved common checks into new `check_array_sort_arguments()` method.
Moved intrinsic's stub presence check to the beginning of methods and added assert to check signature size (number of stack slots) similar to other intrinsics.
I kept `null_check()` together with `stopped()` check for clarity.
Tested locally `java/util/Arrays/Sorting.java` on AVX512 machine to make sure intrinsics are generated and working.
Running tier1-3,stress,xcomp again for this update.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/19918#issuecomment-2195825942
More information about the hotspot-compiler-dev
mailing list