On Tue, 20 Feb 2024 08:36:29 GMT, Emanuel Peter <epe...@openjdk.org> wrote:
>> Jatin Bhateja has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Review comments resolutions. > > src/hotspot/cpu/x86/x86.ad line 4120: > >> 4118: BasicType elem_bt = Matcher::vector_element_basic_type(this); >> 4119: __ lea($tmp$$Register, $mem$$Address); >> 4120: __ vgather8b(elem_bt, $dst$$XMMRegister, $tmp$$Register, >> $idx$$Register, $rtmp$$Register, vlen_enc); > > The `LE8B` and `Matcher::vector_length_in_bytes(n) <= 8` suggest we can > perform this with 4 bytes as well. > Is that correct? > Would that not lead to issues, when we are then reading `base_index` at bytes > 4...7, which possibly have garbage, and then use that to gather? > Do we have tests for that? 64 bit sub-word SPECIES will either hold 8 bytes values or 4 short values, algorithm appropriately handle it. > src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ByteVector.java > line 3071: > >> 3069: .fromArray(lsp, indexMap, mapOffset + i) >> 3070: .add(offset); >> 3071: vix = VectorIntrinsics.checkIndex(vix, a.length); > > are you using the `vix` after this assignment? Its purpose is to check out of bounds indices. > src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ShortVector.java > line 3809: > >> 3807: >> 3808: // Check indices are within array bounds. >> 3809: // FIXME: Check index under mask controlling. > > Did you mean to leave a FIXME? If so, please reference a JIRA bug number > where you intend to fix it. This is in line with already existing FIXME https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/X-Vector.java.template#L5035 ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16354#discussion_r1501753441 PR Review Comment: https://git.openjdk.org/jdk/pull/16354#discussion_r1501753416 PR Review Comment: https://git.openjdk.org/jdk/pull/16354#discussion_r1501753413
