Inefficient horizontal boolean reductions for ARMv7 in Thumb2 mode

[hsivonen]

Steps to reproduce

1. Clone https://github.com/hsivonen/encoding_rs
2. Checkout the simd branch
3. Edit src/mem.rs to change copy_ascii_to_ascii to inline(never)
4. Compile in release mode
5. objdump -d the result
6. Checkout the packed_simd branch
7. Edit src/mem.rs to change copy_ascii_to_ascii to inline(never)
8. Compile in release mode
9. objdump -d the result

Actual results

With the old simd crate, boolean reductions on ARMv7 use vpmax.u8 twice to fold the vector onto itself and then uses vmov.32 once. packed_simd instead uses vmov.32 four times and them ORs them together on the ALU.

simd:

   6c17e:	f921 0a0f 	vld1.8	{d0-d1}, [r1]
   6c182:	ef89 2050 	vshr.s8	q1, q0, #7
   6c186:	ff02 2a03 	vpmax.u8	d2, d2, d3
   6c18a:	ff02 2a00 	vpmax.u8	d2, d2, d0
   6c18e:	ee12 1b10 	vmov.32	r1, d2[0]
   6c192:	2900      	cmp	r1, #0

packed_simd:

   56334:	f961 0a0f 	vld1.8	{d16-d17}, [r1]
   56338:	efc9 2070 	vshr.s8	q9, q8, #7
   5633c:	ee33 1b90 	vmov.32	r1, d19[1]
   56340:	ee32 4b90 	vmov.32	r4, d18[1]
   56344:	ee13 5b90 	vmov.32	r5, d19[0]
   56348:	ee12 6b90 	vmov.32	r6, d18[0]
   5634c:	4321      	orrs	r1, r4
   5634e:	ea46 0405 	orr.w	r4, r6, r5
   56352:	4321      	orrs	r1, r4

Expected results

Expected packed_simd to implement horizontal boolean reductions on ARMv7 the same way as simd.

[gnzlbg]

packed_simd instead uses vmov.32 four times and them ORs them together on the ALU.

Are you using packed_simd with the core_arch feature enabled ? (EDIT: It appears that this is not the case: https://github.com/hsivonen/encoding_rs/blob/packed_simd/Cargo.toml#L17)

[hsivonen]

Only using into_bits from the cargo features of packed_simd.

[gnzlbg]

You need to enable the core_arch (previously called coresimd feature) and compile with -C target-feature=+neon for that to work.

This is because ARMv7 does not support neon, so vpmax.u8 would trigger undefined behavior.

[gnzlbg]

I'm closing this, feel free to re-open if using -C target-feature=+neon and the core_arch cargo feature do not fix this issue for you.

[hsivonen]

I'm not using -C target-feature=+neon with packed_simd because I'm using the thumbv7neon-unknown-linux-gnueabihf target, which was supposed to make a NEON-enabled core::arch available to packed_simd.

When using the core_arch feature, the code uses vpmax.u8 four times per vector and the result is still slower than simd:

packed_simd, inline(never), core_arch
test bench_mem_copy_ascii_to_ascii_1000 ... bench: 146,646 ns/iter (+/- 971) = 3409 MB/s

   6cb36:	f961 0a0f 	vld1.8	{d16-d17}, [r1]
   6cb3a:	efc9 2070 	vshr.s8	q9, q8, #7
   6cb3e:	ff42 2aa3 	vpmax.u8	d18, d18, d19
   6cb42:	ff42 2aa0 	vpmax.u8	d18, d18, d16
   6cb46:	ff42 2aa0 	vpmax.u8	d18, d18, d16
   6cb4a:	ff42 2aa0 	vpmax.u8	d18, d18, d16
   6cb4e:	eed2 1b90 	vmov.u8	r1, d18[0]
   6cb52:	2900      	cmp	r1, #0

[hsivonen]

(GitHub doesn't show me UI to reopen this.)

[hsivonen]

So I think there are two bugs:

1. thumbv7neon-unknown-linux-gnueabihf requires the core_arch feature to use NEON.
2. 4 times vpmax.u8 followed by vmov.u8 instead of 2 times vpmax.u8 followed by vmov.32.

Let's keep this issue about item 2.

[hsivonen]

Filed another issue about item 1.

[gnzlbg]

2. 4 times vpmax.u8 followed by vmov.u8 instead of 2 times vpmax.u8 followed by vmov.32.

That looks bad, I'll investigate.

[gnzlbg]

Which vector type and which reductions are you using ?

[gnzlbg]

m8x16 and using the any reduction ?