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