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

[Emanuel Peter]

On Fri, 24 Oct 2025 09:02:19 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:
> 
>   more tests

The code looks really good now (modulo one tiny suggestion).

I'll run some internal testing now...

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

> 1746:     }
> 1747:     break;
> 1748:   }

Suggestion:


For consistency: let's remove the braces for these cases ;)
You also don't have any below.

test/hotspot/jtreg/compiler/c2/irTests/TestVectorConditionalMove.java line 1526:

> 1524:     @Warmup(0)
> 1525:     @Run(test = {// Signed
> 1526:                  "testCMoveIGTforI",

Thank you very much for adding all the unsigned tests!

We should eventually also add more signed tests. Are you planning on doing that anyway in the future? Either way: we should have an RFE, and link it to this bug here. If you don't want to work on it, then please assign it to me ;)

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

PR Review: https://git.openjdk.org/jdk/pull/27942#pullrequestreview-3387682395
PR Review Comment: https://git.openjdk.org/jdk/pull/27942#discussion_r2468737088
PR Review Comment: https://git.openjdk.org/jdk/pull/27942#discussion_r2468750039