RFR: 8370481: C2 SuperWord: Long/Integer.compareUnsigned return wrong value in SLP [v3]

[Emanuel Peter]

On Thu, 23 Oct 2025 13:44:41 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Hi,
>> Can you help to review the patch? @eme64 
>> 
>> ##  Issue
>> 
>> Currently, in SLP when transform from (Bool + CmpU + CMove) to (VectorMaskCmp + VectorBlend), the unsigned-ness in CmpU is lost, then end up doing a signed instead of unsigned comparison in VectorMaskCmp.
>> For details please check code at `SuperWordVTransformBuilder::make_vector_vtnode_for_pack` and `PackSet::get_bool_test`.
>> 
>> ##  Fix
>> Currently, `BoolTest` does not support an unsigned construction (`BoolTest( mask btm ) : _test(btm) { assert((btm & unsigned_compare) == 0, "unsupported");}`), seems to me a feasible solution would be get the unsigned information from CmpU (which could be an input of Bool) and pass it to VectorMaskCmp.
>> 
>> Thanks
>> 
>> This pr could also lead to more optimizations, like: https://github.com/openjdk/jdk/pull/25336 and https://github.com/openjdk/jdk/pull/25341.
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add all possible Cmp cases; add comments

src/hotspot/share/opto/superword.cpp line 1752:

> 1750:   case Op_CmpUL:
> 1751:   case Op_CmpU3:
> 1752:   case Op_CmpUL3:

Do these really ever survive to here? I thought they should all be turned into other ops? Do you have an example?

src/hotspot/share/opto/superword.cpp line 1758:

> 1756:     break;
> 1757:   case Op_CmpN:
> 1758:   case Op_CmpP:

Did you ever encounter these? I'd be surprised if these work... Because they work with pointers, and we don't support pointers, only primitives.

src/hotspot/share/opto/superword.cpp line 1760:

> 1758:   case Op_CmpP:
> 1759:   case Op_CmpD3:
> 1760:   case Op_CmpF3:

These would probably have the same issue as the regular `CmpF/CmpD`, no? So are we sure we can "allow-list" them here?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27942#discussion_r2455362948
PR Review Comment: https://git.openjdk.org/jdk/pull/27942#discussion_r2455361259
PR Review Comment: https://git.openjdk.org/jdk/pull/27942#discussion_r2455365835