8387812: Refine ArraysSupport.vectorizedMismatch to compare all elements#31802
8387812: Refine ArraysSupport.vectorizedMismatch to compare all elements#31802kuaiwei wants to merge 2 commits into
Conversation
|
👋 Welcome back kwei! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@kuaiwei The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
|
The total number of required reviews for this PR has been set to 2 based on the presence of this label: |
Webrevs
|
|
ArraysSupport.vectorizedMismatch JavaDoc says the following: Unless you change the contract (so it is mandated to check all elements), I don't see how you can simplify checks at call sites. But another question is do we really want to do so? It forces intrinsics on all platforms to fully process input arrays which may introduce unnecessary complexity. Alternatively, you can introduce a wrapper which handles tail processing in Java. |
Thanks for the review. A couple of clarifications: The proposed change does not break the existing contract. If all elements are checked and no mismatch is found, returning -1 is fully consistent with the current JavaDoc specification. Existing call sites continue to work correctly without any modification — the changes to current JDK call sites are merely simplifications that take advantage of the stronger guarantee. In practice, x86_64 is the only platform that provides an intrinsic for this method today, and it already processes all elements. I noticed this behavioral discrepancy while working on a RISC-V intrinsic for it. From the RISC-V perspective, scanning all elements is actually the simpler approach to implement. |
How do you get stronger guarantees? Is it based solely on implementation-specific observations that the intrinsics always perform full scan over the arrays? If |
|
The call sites I modified are within the OpenJDK codebase itself, so they can be updated in the same PR to match the new spec. To clarify the compatibility concern, here's the typical pattern for old spec: int i = vectorizedMismatch(...);
if (i >= 0) {
return i; // mismatch found
} else {
i = length - ~i;
}
for (; i < length; i++) {
if (a[i] != b[i])
return i;
}
return -1; // no mismatchUnder the new behavior, vectorizedMismatch returns -1. This gives i = length - ~(-1) = length, so the |
|
Hi @iwanowww, I think the spec can remain unchanged. The current spec does not limit a full scan or a partial scan — this is implementation-dependent. So the proposal is simply to align the Java implementation with the existing x86_64 intrinsic behavior, making them consistent without any spec changes. How do you think about it ? |
I recently noticed a behavioral discrepancy in jdk.internal.util.ArraysSupport.vectorizedMismatch between the Java implementation and the platform intrinsic implementations.
Current behavior
The Java implementation may leave a tail of elements unchecked, returning the bitwise complement of the number of remaining elements (i.e., ~remaining).
The x86_64 intrinsic, by contrast, compares all elements and simply returns -1 when no mismatch is found.
Because of this inconsistency, every caller has to handle the "remaining elements" case defensively:
Proposed change
This PR refines the Java implementation so that it always compares all elements and returns -1 when no mismatch is found, matching the x86_64 intrinsic behavior. This also simplify all callsite because it eliminates the need for callers to handle remaining elements.
A regression test is included at
test/hotspot/jtreg/compiler/intrinsics/VectorizedMismatchReturnDiffTest.javawhich demonstrates the original behavioral difference.Test
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/31802/head:pull/31802$ git checkout pull/31802Update a local copy of the PR:
$ git checkout pull/31802$ git pull https://git.openjdk.org/jdk.git pull/31802/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 31802View PR using the GUI difftool:
$ git pr show -t 31802Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/31802.diff
Using Webrev
Link to Webrev Comment