RFR: 8371187: [BigEndian Platforms] Vector lane reversal error [v3]

Martin Doerr mdoerr at openjdk.org
Tue Jan 20 18:58:54 UTC 2026


On Tue, 20 Jan 2026 17:12:15 GMT, Varada M <varadam at openjdk.org> wrote:

>> This change fixes incorrect lane ordering in reinterpretation operations on big-endian platforms. When converting from wider to narrower lane types like Long to Int, Long to Short, big-endian systems produced reversed sub-lanes.
>> The patch adds a maybeSwapOnConverted() and a generic normalizeSubLanesForSpecies() shuffle builder to correct the sub-lane order based on element sizes on big-endian
>> 
>> JBS: [JDK-8371187](https://bugs.openjdk.org/browse/JDK-8371187)
>
> Varada M has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains two new commits since the last revision:
> 
>  - 8371187: [BigEndian Platforms] Vector lane reversal error
>  - fix for vector alignment issue on big-endian

Thanks! I have more questions and suggestions.

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/AbstractVector.java line 185:

> 183:         // NOTE:  This assumes that convert0('X')
> 184:         // respects REGISTER_ENDIAN order.
> 185:         return convert0('X', vspecies().withLanes(laneType)).maybeSwapOnConverted(java.nio.ByteOrder.nativeOrder(), vspecies());

Would `swapIfNeeded` be a better name?
I think we could determine the ByteOrder where it is used. Then, we don't have to pass it any more.

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/AbstractVector.java line 194:

> 192:             int[] id = new int[lanes];
> 193:             for (int i = 0; i < lanes; ++i) id[i] = i;
> 194:             return VectorShuffle.fromArray(targetSpecies, id, 0);

Some comments would be helpful describing what you do in which case.

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/AbstractVector.java line 211:

> 209: 
> 210:     @ForceInline
> 211:     final int subLanesPerSrcIfNeeded(ByteOrder bo, AbstractSpecies<?> srcSpecies) {

Would `subLanesToSwap` be a better name?
Shouldn't it be `protected`, too?

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/AbstractVector.java line 217:

> 215:         int sBytes = srcSpecies.elementSize();
> 216:         int tBytes = vspecies().elementSize();
> 217:         if (sBytes == tBytes || (sBytes % tBytes) != 0) {

What do we do if it's not divisible? Should we better throw an exception?

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

PR Review: https://git.openjdk.org/jdk/pull/28425#pullrequestreview-3683652415
PR Review Comment: https://git.openjdk.org/jdk/pull/28425#discussion_r2709564720
PR Review Comment: https://git.openjdk.org/jdk/pull/28425#discussion_r2709629154
PR Review Comment: https://git.openjdk.org/jdk/pull/28425#discussion_r2709553616
PR Review Comment: https://git.openjdk.org/jdk/pull/28425#discussion_r2709673998


More information about the core-libs-dev mailing list