On Tue, 20 Jan 2026 17:12:15 GMT, Varada M <[email protected]> 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
