Repository navigation
Add NEON for av2_trellis_loop_diagonal_st8 - #5489
jjustiss-apple wants to merge 3 commits into
Conversation
5ff0cbf to
61d364d
Compare
| memcpy(&eob_packed, &cost_eob_tbl[idx][eob_ctx][0], 4); | ||
| uint32x4_t eob_wide = | ||
| vmovl_u16(vreinterpret_u16_u32(vcreate_u32(eob_packed))); | ||
| int32x2_t rate_eob = vadd_s32(vget_low_s32(vreinterpretq_s32_u32(eob_wide)), |
There was a problem hiding this comment.
In decide_states_q1_neon_impl (which is called after this function) it looks like only rate_eob[1] is used. so it's only necessary to calculate rate_eob[1] here.
Same in get_rate_dist_lf_luma_q1_neon_impl
There was a problem hiding this comment.
You're right, updated.
| }; | ||
| // clang-format on | ||
|
|
||
| static AVM_FORCE_INLINE void load_unpack_lf_base_cost( |
There was a problem hiding this comment.
These 4 functions load_unpack_lf_*_cost and load_unpack_*_cost are almost the same. Can they be consolidated into one helper function?
There was a problem hiding this comment.
I agree a macro is cleaner, updated
| int a0_0 = (i & 2) ? 1 : 0; | ||
| int a0_1 = ((i + 1) & 2) ? 1 : 0; |
There was a problem hiding this comment.
i is always even so a0_0 and a0_1 are always equal
There was a problem hiding this comment.
you're right, updated. dropped a0_1 and changed a0_0 to a0
| int a0_0 = (i & 2) ? 1 : 0; | ||
| int a0_1 = ((i + 1) & 2) ? 1 : 0; | ||
|
|
||
| uint32x2_t r_ev = vcreate_u32( |
There was a problem hiding this comment.
These are recreating from scalar right? rate and rz and re are loaded to scalar. they can be calculated without that? rate[2 * i] and rate[2 * (i + 1)] are in continuous memory. for example:
uint32x2x2_t r_pair = vld2_u32((const uint32_t *)&rate[2 * i]);
uint32x2_t r_ev = r_pair.val[0];
uint32x2_t r_od = r_pair.val[1];
There was a problem hiding this comment.
Updated, good catch.
| rcz[i + 1] = vgetq_lane_s64(c_zr, 1); | ||
| } | ||
|
|
||
| node_general(rc[0], rc[1], rcz[0], rate[0], rate[1], rz[0], pq->absLevel[0], |
There was a problem hiding this comment.
these are all scalar operations that contain some branches in the function. Each pair of state (i, i+1) fits in 2-lane 64bit vectors
Converting the conditions in node_general into SIMD can be beneficial:
Compare Zero vs. Even with vcgtq_s64(c_ev, c_zr) and blend with vbslq.
Swap the 64-bit lanes of the Odd candidate (vextq_s64(c_od, c_od, 1)).
Compare Even/Zero vs. Odd with vcgtq_s64(c_ev, c_od_swapped) and blend with vbslq.
There was a problem hiding this comment.
I'm certainly open to incorporating your vectorizing idea, but I've been writing strictly armv7a code thus far. I believe vcgtq_s64 is an aarch64/armv8a instruction.
@yunqingwang1 do we want to consider relaxing the armv7a NEON requirement?
| } | ||
| tcq_rate_t rd; | ||
|
|
||
| if (UNLIKELY(pq_data.orig_qIdx < 2)) { |
There was a problem hiding this comment.
Is there any evidence for this?
There was a problem hiding this comment.
This was speculative and didn't hold up to ablation testing, I removed it.
…ate_eob lane, simplify decide_states loads
3 participants
Full ARMv7-A NEON port of the TCQ trellis diagonal loop. Fuses all
sub-kernels (pre_quant, rate_dist, decide_states, update_states,
update_nbr_diagonal) into a single inlined loop, vectorizing quantization,
RD-cost computation, context extraction, and state update with NEON
intrinsics. The decide_states path remains scalar (no vcgtq_s64 on ARMv7)
but uses ccmp chains for efficient 3-way branching.
Also consolidates duplicated helpers (kGolombExp0Bits, get_dqv,
get_diag_ctx) from NEON/AVX2/C into trellis_quant.h.
CTC results (RA, cpu-used=1, 33 frames, A4+A5):
Micro-benchmarks (Apple Silicon M2 Ultra, total_us/coeffs, 100M iterations):
Unit tests included (RandomValues + Speed).