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

Jatin Bhateja jbhateja at openjdk.org
Thu Sep 19 06:44:20 UTC 2024


On Wed, 18 Sep 2024 14:22:16 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

> > > Do we have tests for the publically exposed methods in this new file? Or are they only implicitly tested through the VectorAPI, and its tests? `src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorMath.java`
> > > Why is this even called `VectorMath`? Because those ops are not at all restricted to vectorization, right?
> > 
> > 
> > Nomenclature is suggested by Paul.
> 
> @PaulSandoz Do you want to limit these **scalar** operations to a class name that implies **vector** use?
> 
> > We have sufficient test coverage of these APIs in JTREG tests.
> 
> @jatin-bhateja I can't see any dedicated JTREG tests to the `VectorMath` methods. I only see the VectorAPI tests. Can you point me to the `VectorMath` tests? I'd like to review them.

Hi @eme64 , @PaulSandoz  
Yes dedicated test for each of newly added VectorMath operation is justified here.

Thanks, let me know if there are other comments.

 

> > > > > Why is this even called `VectorMath`? Because those ops are not at all restricted to vectorization, right?
> > > > 
> > > > 
> > > > Nomenclature is suggested by Paul.
> > > 
> > > 
> > > @PaulSandoz Do you want to limit these **scalar** operations to a class name that implies **vector** use?
> > 
> > 
> > It's whatever math functions are required to in support of vector operations (as the JavaDoc indicates) that are not provided by other classes such as the boxed primitives or `java.lang.Math`.
> 
> Ok. I suppose these methods could eventually be moved to `java.lang.Math` or some other `java.lang` class, when the VectorAPI goes out of incubator mode?
> 
> I feel like these saturating operations, and also the unsigned ops could find a more wider use, away from (explicit) vector usage. For example, the saturating operations are nice because they prevent overflows, and in some cases that would be very nice to have readily available.

Hi @eme64 , yes that what our extended plan is, for this patch we want to restrict its use to VectorAPI. 

> > > Do we have tests for the publically exposed methods in this new file? Or are they only implicitly tested through the VectorAPI, and its tests? `src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorMath.java`
> > > Why is this even called `VectorMath`? Because those ops are not at all restricted to vectorization, right?
> > 
> > 
> > Nomenclature is suggested by Paul.
> 
> @PaulSandoz Do you want to limit these **scalar** operations to a class name that implies **vector** use?
> 
> > We have sufficient test coverage of these APIs in JTREG tests.
> 
> @jatin-bhateja I can't see any dedicated JTREG tests to the `VectorMath` methods. I only see the VectorAPI tests. Can you point me to the `VectorMath` tests? I'd like to review them.

@eme64 , @PaulSandoz , I agree that explicit test for all newly added VectorMath operation for all integral types is justified here.

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

PR Comment: https://git.openjdk.org/jdk/pull/20507#issuecomment-2360118143


More information about the core-libs-dev mailing list