On Thu, 13 Aug 2026 22:48:49 GMT, Chad Rakoczy <[email protected]> wrote:
> Adds support for vectorized dot product on aarch64 (`sdot` and `udot`) > through the Vector API > > Dedicated dot product instructions have been shown to provide up to 10x > throughput for lucene > ([results](https://github.com/apache/lucene/pull/13572)) compared to > vectorized multiply and add. This PR adds two new methods to `ByteVector` to > leverage the aarch dot product instructions `sdot` and `udot` respectively. > - `IntVector dot(Vector<Byte> v, Vector<Integer> acc)` > - `IntVector dotUnsigned(Vector<Byte> v, Vector<Integer> acc)` > > Each int lane of the result holds the dot product of the corresponding group > of four bytes from the two operands, added to the matching accumulator lane. > For example: > > a = [a1, a2, a3, a4, ..., ..., a13, a14, a15, a16] > b = [b1, b2, b3, b4, ..., ..., b13, b14, b15, b16] > acc = [acc1, ..., ..., acc4] > > a.dot(b, acc) -> [ > acc1 + a1 * b1 + a2 * b2 + a3 * b3 + a4 * b4, > ..., > ..., > acc4 + a13 * b13 + a14 * b14 + a15 * b15 + a16 * b16 > ] > > > The equivalent instructions on x86 are `VPDPBSSD` and `VPDPBUUD` which > perform the same operations and match the proposed new functions however this > PR only includes aarch64. > > Graviton 2 > > Benchmark Mode Cnt Score Error Units > VectorDotBenchmark.dotScalar thrpt 25 2412.719 ± 0.014 ops/ms > VectorDotBenchmark.dotMulAdd thrpt 25 1497.953 ± 2.734 ops/ms > VectorDotBenchmark.dotVector thrpt 25 23243.016 ± 141.708 ops/ms > VectorDotBenchmark.dotUnsignedScalar thrpt 25 2402.966 ± 0.157 ops/ms > VectorDotBenchmark.dotUnsignedMulAdd thrpt 25 1498.898 ± 1.442 ops/ms > VectorDotBenchmark.dotUnsignedVector thrpt 25 23344.116 ± 222.150 ops/ms > > > Graviton 3 > > Benchmark Mode Cnt Score Error Units > VectorDotBenchmark.dotScalar thrpt 25 7946.950 ± 2.564 ops/ms > VectorDotBenchmark.dotMulAdd thrpt 25 3257.311 ± 5.308 ops/ms > VectorDotBenchmark.dotVector thrpt 25 41430.996 ± 688.936 ops/ms > VectorDotBenchmark.dotUnsignedScalar thrpt 25 2536.268 ± 0.438 ops/ms > VectorDotBenchmark.dotUnsignedMulAdd thrpt 25 3259.853 ± 3.511 ops/ms > VectorDotBenchmark.dotUnsignedVector thrpt 25 41263.821 ± 478.856 ops/ms > > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). I left comments that focus on the implementation part. Thanks! src/hotspot/cpu/aarch64/aarch64_vector.ad line 202: > 200: if (VM_Version::use_neon_for_vector(length_in_bytes) && > !VM_Version::supports_asimddp()) { > 201: return false; > 202: } Could you please add a comment about the limitation? src/hotspot/cpu/aarch64/aarch64_vector.ad line 8102: > 8100: $src2$$FloatRegister, $src3$$FloatRegister, > 8101: length_in_bytes == 16 ? __ T16B : __ T8B); > 8102: } else { Do we need to consider the long/short types, which the SVE instruction supports while NEON doesn't support it? Although it doesn't contains these APIs now, we'd better to consider it in the codegen in case it is supported in future. At least, I think an assertion is deserved here. src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 4377: > 4375: INSN(sdot, /* is_unsigned */ false); > 4376: INSN(udot, /* is_unsigned */ true); > 4377: #undef INSN Is it better to move it to the place where other NEON instructions are defined? src/hotspot/share/opto/vectorIntrinsics.cpp line 2983: > 2981: NodeClassNames[argument(3)->Opcode()]); > 2982: return false; > 2983: } Suggest to align the code style with other intrinsifics. Suggestion: const TypeInt* opr = gvn().type(argument(0))->isa_int(); const TypeInstPtr* from_vector_klass = gvn().type(argument(1))->isa_instptr(); const TypeInt* from_lane_type = gvn().type(argument(2))->isa_int(); const TypeInt* from_vlen = gvn().type(argument(3))->isa_int(); if (opr == nullptr || !opr->is_con() || from_vector_klass == nullptr || from_vector_klass->const_oop() == nullptr || from_lane_type == nullptr || !from_lane_type->is_con() || from_vlen == nullptr || !from_vlen->is_con()) { log_if_needed(" ** missing constant: opr=%s vclass=%s etype=%s vlen=%s", NodeClassNames[argument(0)->Opcode()], NodeClassNames[argument(1)->Opcode()], NodeClassNames[argument(2)->Opcode()], NodeClassNames[argument(3)->Opcode()]); return false; // not enough info for intrinsification } int op = opr->get_con(); int vop; if (op == VectorSupport::VECTOR_OP_DOT) { vop = Op_DotV; } else if (op == VectorSupport::VECTOR_OP_UDOT) { vop = Op_UDotV; } else { log_if_needed(" ** unsupported operation id: %d", op); return false; } ... src/hotspot/share/opto/vectorIntrinsics.cpp line 3048: > 3046: > 3047: const TypeVect* vt = TypeVect::make(to_elem_bt, to_num_elem); > 3048: Node* node = gvn().transform(VectorNode::make(op, opd1, opd2, opd3, > vt)); Suggest to add `trace_vector()` for new created vector node. src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ByteVector.java line 2650: > 2648: */ > 2649: @ForceInline > 2650: public final IntVector dot(Vector<Byte> v, Vector<Integer> acc) { Is it clearer defining this API in `IntVector` which accepts two byte vectors and use `this` int vector as the accumulator vector? ------------- PR Review: https://git.openjdk.org/jdk/pull/32359#pullrequestreview-5581889737 PR Review Comment: https://git.openjdk.org/jdk/pull/32359#discussion_r4322425233 PR Review Comment: https://git.openjdk.org/jdk/pull/32359#discussion_r4322415636 PR Review Comment: https://git.openjdk.org/jdk/pull/32359#discussion_r4322438939 PR Review Comment: https://git.openjdk.org/jdk/pull/32359#discussion_r4322565708 PR Review Comment: https://git.openjdk.org/jdk/pull/32359#discussion_r4322601156 PR Review Comment: https://git.openjdk.org/jdk/pull/32359#discussion_r4322721585
