Vector API review

[John Rose]

Thanks for the read-over and the careful review.

I disagree with most of your suggestions, even though
they are reasonable.  This is a good moment to explain
why some things are the way they are.

On Mar 31, 2021, at 2:43 PM, Remi Forax <forax at univ-mlv.fr> wrote:
> 
> Hi all,
> next week i will do a lab on the vector API, so i've played a little more with the API.
> 
> There are still things to polish,
> 
> - IntVector.addIndex() should be addLaneIndex(int scale)

Good.

> - Vector.lanewise should be Vector.apply given that test or conversion are also lanewise,
>  calling a function is usually "apply".

(Already considered and rejected.)

The most important fact is not that the op is a function,
but rather that the vector has lanes.  Think of “lanewise”
as a graceful abbreviation of “applyLanewise”.

> - While Vector.lanewise is overloaded for unary, binary, ternary and associative,
>  for unary test and binary test, we have two different name test and compare.
>  I think, compare should be renamed to test.
>  And VectorOperators.Test should be VectorOperators.UnaryTest and Comparison should be BinaryTest.

(Already considered and rejected.)

You test one thing and you compare two things.

In this case it also could have been called lanewise, but
making it return a VectorMask would have required tricky
overload stuff.

>  I still don't like Associative, it seems risky to have a type just for reduceLane.

(Already considered and rejected.)

Associative lets you lump “add-used-for-reduction” with “add-used-elsewhere”.
That’s better than splitting out two versions of ADD (“what’s AADD, teacher?”).

>  What if in the future Intel, AMD, ARM or whatever decide that reduceLane can work with SUB ?
Then we would split out another version of SUB, ASUB.  But it won’t happen.

>  In term of javadoc, all the intergace like VectorOperations.Unary, Binary, etc should document all the possible operations,
>  instead we have to crawle VectorOperations looking for the right type.

Maybe.  Although the IDE will do that for you during completion,
because of the typefulness of that argument position.

>  There is no iota as a static method on IntVector, LongVector, etc, so we have to use zero().addIndex(1) which is weird
>  (or i've missed something ?)

No, it’s just an arbitrary choice to leave the user to compose
addIndex with a constant zero.

> - Having add, sub, etc on Vector is useless given that we already have lanewise().

That’s a conscious decision, documented in the package-info.
The universal API is lanewise (and test/compare), but the basic
full-support operations *also* have named methods.  Surely
you can see this is designed to be pedagogically helpful.
Not useless at all, just not functionally minimal.

>  Having add, etc, etc on the specialized version, IntVector, LongVector, is a better idea because the parameter type can be typed
>  with the right specialized type instead of Vector, e.g, IntVector.add(IntVector) instead of InteVector.add(Vector) 

(That’s true in general of lanewise also.)

> - Can wa agree that having a static method and an instance method with the same name like broadcast is not a good idea

In general, yes.  But broadcast is very special.

>  I don't see the point of the instance method given that it replaces all the values.

This is the point:

var v = (something configuration sensitive);
var w = v.broadcast(42);
(do stuff with v and w)

You need a graceful way to break up stuff like this:

v.lanewise(op, 42);

into stuff like this:

var k42 = v.broadcast(42);
…more stuff added...
v.lanewise(op, k42);

We must not assume that the static type of v is
always readily available, to invoke a static factory.

> - I aslo don't see the point of maskAll() given that it's species().maskAll()

Pedagogy.  But I agree it’s a slippery slope.

> - I don't see why convertShape is not named convert too given that convert also returns a vector with a different shape

The conversions are different enough that they need
different names to avoid confusion.  Shape changes
are tricky and may have extra cost, so need to be
called out explicitly.

I don’t see how plain convert changes shape…?

> - remove selectFrom() given that it's rearrange()

We added that for the about same reason some ISAs
have both SUB and RSUB.  There are use cases for
either, and it’s artificial to require minimality here.

> - I dont see the point of the reinterpretAsFoo, perhaps a reinterpretAs(Class<?>) is enough ?

No, it’s not.  The static return type is important, and
cannot be derived polymorphically from a Class arg.

> 
> - check() can renamed to "is"/"isInstance" because it's an instanceof test

Not within the vector paradigm.  Tests are lanewise and generate masks.
If you really want an instanceof test, use the instanceof operator,
but that isn’t part of the vector programming model.

In any case, it’s a cast not a test.  It is sometimes useful to re-derive
static type information (in fluent syntax), after an API point has lost
that information.

— John