RFR: 8350177: C2 SuperWord: Integer.numberOfLeadingZeros, numberOfTrailingZeros, reverse and bitCount have input types wrongly turncated for byte and short [v4]
Emanuel Peter
epeter at openjdk.org
Wed Jun 18 07:45:38 UTC 2025
On Wed, 18 Jun 2025 04:02:36 GMT, Jasmine Karthikeyan <jkarthikeyan at openjdk.org> wrote:
>> Hi all,
>> This patch fixes cases in SuperWord when compiling subword types where vectorized code would be given a narrower type than expected, leading to miscompilation due to truncation. This fix is a generalization of the same fix applied for `Integer.reverseBytes` in [JDK-8305324](https://bugs.openjdk.org/browse/JDK-8305324). The patch introduces a check for nodes that are known to tolerate truncation, so that any future cases of subword truncation will avoid creating miscompiled code.
>>
>> The patch reuses the existing logic to set the type of the vectors to int, which currently disables vectorization for the affected patterns entirely. Once [JDK-8342095](https://bugs.openjdk.org/browse/JDK-8342095) is merged and automatic casting support is added the autovectorizer should automatically insert casts to and from int, maintaining correctness.
>>
>> I've added an IR test that checks for correctly compiled outputs. Thoughts and reviews would be appreciated!
>
> Jasmine Karthikeyan has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>
> - Merge, address code review
> - Merge branch 'master' into vector-truncation
> - Add assert for unexpected node in truncation
> - Reformat, add comments and char tests
> - Fix vector truncation with subword types
src/hotspot/share/opto/superword.cpp line 2549:
> 2547:
> 2548: // For shorts and chars, check an additional set of nodes.
> 2549: if (type->isa_int() == TypeInt::SHORT || type->isa_int() == TypeInt::CHAR) {
What is this change for?
src/hotspot/share/opto/superword.cpp line 2594:
> 2592: // and then add it to the list of truncating nodes or the list of non-truncating ones just above.
> 2593: // In product, we just return false, which is always correct.
> 2594: assert(false, "Unexpected node in SuperWord truncation: %s", NodeClassNames[in->Opcode()]);
Suggestion:
// If this assert is hit, that means that we need to determine if the node can be safely truncated,
// and then add it to the list of truncating nodes or the list of non-truncating ones just above.
// In product, we just return false, which is always correct.
assert(false, "Unexpected node in SuperWord truncation: %s", NodeClassNames[in->Opcode()]);
Suggestion:
// If this assert it hit, that means that we need to determine if the node can be safely truncated,
// and then add it to the list of truncating nodes or the list of non-truncating ones just above.
// In product, we just return false, which is always correct.
assert(false, "Unexpected node in SuperWord truncation: %s", NodeClassNames[in->Opcode()]);
Probably my bad previous suggestion is the culprit here 🙈
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25440#discussion_r2153885586
PR Review Comment: https://git.openjdk.org/jdk/pull/25440#discussion_r2153880820
More information about the hotspot-compiler-dev
mailing list