Skip to content

ext/standard: Remove useless _mm_movemask_epi8 check in rot13 - #23795

Open
LamentXU123 wants to merge 1 commit into
php:masterfrom
LamentXU123:rot13
Open

LamentXU123 wants to merge 1 commit into
php:masterfrom
LamentXU123:rot13

Conversation

@LamentXU123

@LamentXU123 LamentXU123 commented Sep 20, 2026

Copy link
Copy Markdown
Member

The _mm_movemask_epi8 if-branch seems useless. From comparing, the only possible cases for each byte is 0x00 and 0xFF. So just perform the and and or command anyways is always correct.

When the mask aren't all zero, removing them is certainly faster cuz we save a _mm_movemask_epi8 call. When it is all zero, from my limited knowledge I think the pand plus por command is always faster than pmovmskb (plus test + jcc if that counts). But I might be wrong.
Here is the asm diff:

Orginal

pmovmskb eax, xmm1
test     eax, eax
je       .L4
pand     xmm1, xmm12
por      xmm2, xmm1
.L4:

This version

pand     xmm1, xmm12
por      xmm2, xmm1

So. I think this if-branch "optimization" actually made things slower. Note that we are checking this four times in a loop.

I tested this on my personal PC and the improvements are quite stable. ~1.2x-1.3x and in the worse case I don't see anything negative appears. But note that my env is kind of noisy. I don't know much on this perhaps I need help from some expert. @ndossche ?

Comment thread ext/standard/string.c
Comment on lines 6037 to +6059
gt = _mm_cmpgt_epi8(in, a_minus_1);
lt = _mm_cmplt_epi8(in, m_plus_1);
cmp = _mm_and_si128(lt, gt);
if (_mm_movemask_epi8(cmp)) {
cmp = _mm_and_si128(cmp, add);
delta = _mm_or_si128(delta, cmp);
}
cmp = _mm_and_si128(cmp, add);
delta = _mm_or_si128(delta, cmp);

gt = _mm_cmpgt_epi8(in, n_minus_1);
lt = _mm_cmplt_epi8(in, z_plus_1);
cmp = _mm_and_si128(lt, gt);
if (_mm_movemask_epi8(cmp)) {
cmp = _mm_and_si128(cmp, sub);
delta = _mm_or_si128(delta, cmp);
}
cmp = _mm_and_si128(cmp, sub);
delta = _mm_or_si128(delta, cmp);

gt = _mm_cmpgt_epi8(in, A_minus_1);
lt = _mm_cmplt_epi8(in, M_plus_1);
cmp = _mm_and_si128(lt, gt);
if (_mm_movemask_epi8(cmp)) {
cmp = _mm_and_si128(cmp, add);
delta = _mm_or_si128(delta, cmp);
}
cmp = _mm_and_si128(cmp, add);
delta = _mm_or_si128(delta, cmp);

gt = _mm_cmpgt_epi8(in, N_minus_1);
lt = _mm_cmplt_epi8(in, Z_plus_1);
cmp = _mm_and_si128(lt, gt);
if (_mm_movemask_epi8(cmp)) {
cmp = _mm_and_si128(cmp, sub);
delta = _mm_or_si128(delta, cmp);
}
cmp = _mm_and_si128(cmp, sub);
delta = _mm_or_si128(delta, cmp);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c | 0x20 maps A-Z onto a-z and moves nothing else into that range (it only touches bit 5, and bytes >= 0x80 stay negative under the signed compare), so the four range tests collapse into one plus a midpoint test:

const __m128i lowbit = _mm_set1_epi8(0x20);
const __m128i a_minus_1 = _mm_set1_epi8('a' - 1);
const __m128i z_plus_1 = _mm_set1_epi8('z' + 1);
const __m128i n = _mm_set1_epi8('n');
const __m128i add = _mm_set1_epi8(13);
const __m128i sub = _mm_set1_epi8(-13);

do {
	__m128i in = _mm_loadu_si128((__m128i *)p);
	__m128i low = _mm_or_si128(in, lowbit);
	__m128i alpha = _mm_and_si128(_mm_cmpgt_epi8(low, a_minus_1), _mm_cmplt_epi8(low, z_plus_1));
	__m128i first = _mm_cmplt_epi8(low, n);
	__m128i delta = _mm_and_si128(_mm_or_si128(_mm_and_si128(first, add), _mm_andnot_si128(first, sub)), alpha);

	_mm_storeu_si128((__m128i *)target, _mm_add_epi8(in, delta));

	p += 16;
	target += 16;
} while (e - p > 15);

gcc 11.4 -O2: the compiled loop body goes from 28 SIMD instructions to 15, and the 4-deep serial OR chain on delta goes away. Best of 7 over 1 MiB against your version: 1.9x on all-lowercase, 1.9x on digits, 2.4x on mixed case. Byte-identical to the scalar fallback for every byte value in every lane.

Separately, for the description: 1.2-1.3x is the floor, not the typical case. Random bytes at 1 MiB measure 7.9x against master here, since the four branches mispredict. Small buffers hide that, the predictor just memorizes a short repeating pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants