RFR: 8338021: Support new unsigned and saturating vector operators in VectorAPI [v31]

Jatin Bhateja jbhateja at openjdk.org
Thu Oct 24 13:36:51 UTC 2024


On Thu, 24 Oct 2024 06:31:00 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorMath.java line 32:
>> 
>>> 30:  * @since   24
>>> 31:  */
>>> 32: public final class VectorMath {
>> 
>> I think this class could have been split into a separate RFE, together with its tests. I would prefer that next time.
>
> Also: why did we not add these `Long.minUnsigned` etc? I guess that was already discussed?
> Because we can easily also use this with the auto-vectorizer or more generally. Saturating and unsigned ops are generally useful I think...

PR is specially targeting explicit vectorization flow, we plan to address scalar intrinsification and auto-vectorization later, once type system has exposure to unsigned types.

>> test/jdk/jdk/incubator/vector/templates/Kernel-SaturatingBinary-Masked-op.template line 8:
>> 
>>> 6: 
>>> 7:         for (int ic = 0; ic < INVOC_COUNT; ic++) {
>>> 8:             for (int i = 0; i < a.length; i += SPECIES.length()) {
>> 
>> I think this does not check if the generated vectors are too long. We had bugs in the past where we should have created say 2-element vectors, but the backend wrongly created 4-element vectors. This is especially an issue with vectors that do direct memory access.
>> 
>> With a simple "counting-up" test, you will probably not catch this. It could be good to have a "counting-down" example as well. What do you think?
>
> Also: all of these cases load, and directly store again. Does that not mean all tests will probably pick the "..._mem" backend operations? Or do we actually end up testing all backend operations with the tests we have here?

To exercise non memory operand pattern we need a vector operation padding layer after load vector, this will always ensure that selector pick all register operands flavor of instruction. Since its a generic limitation, do you think we should float it as a separate PR? 

I have create an RFE https://bugs.openjdk.org/browse/JDK-8342959  for reference. Given that we have moved IR tests out this PR on the grounds of review complexity, lets not add more code here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20507#discussion_r1815000046
PR Review Comment: https://git.openjdk.org/jdk/pull/20507#discussion_r1814998821


More information about the hotspot-compiler-dev mailing list