RFR: 8318509: x86 count_positives intrinsic broken for -XX:AVX3Threshold=0 [v2]

[Claes Redestad]

On Tue, 24 Oct 2023 08:57:10 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Claes Redestad has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Remove redundant 0 + tail
>>  - Randomize array segment sizes and the amount of negatives. Add some tests that limit negatives to a tail.
>
> test/hotspot/jtreg/compiler/intrinsics/string/TestCountPositives.java line 130:
> 
>> 128:         int expected = countPositives(bytes, off, len);
>> 129:         if (calculated != expected) {
>> 130:             if (expected != len && ng >= 0 && calculated >= 0 && calculated < expected) {
> 
> Why does this happen? Why does `countPositives` not get the same result here?
> (I see that this was here before)

The `countPositives` intrinsics were adapted from the pre-existing `hasNegatives` intrinsic, and to ease porting over all platforms - including those we don't maintain - we specified `countPositives` so that if there's a negative value at index `idx` then it's acceptable for the intrinsics to return any value from `0` to `idx - 1`. This way intrinsics on all platforms could quickly be adapted over (return either `0` for `hasNegatives == true` or `length` for `hasNegatives == false`) while respective maintainers worked on porting over properly. 

This also allows the intrinsic to do a quick approximation: E.g. if a vectorized loop has checked 128 bytes but then detects a negative in the next block then the intrinsic can opt to return 128 rather than whatever the exact count. The surrounding java code will then do a scalar loop that is roughly as fast as an intrinsic tail routine would be, and with less complexity. I believe the `aarch64` intrinsic is implemented like this.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16267#discussion_r1369878428