Repository navigation
Add some LoongArch fixes - #1457
Merged
Merged
Conversation
SIMDE_LOONGARCH_LSX_NATIVE and SIMDE_LOONGARCH_LASX_NATIVE were defined at the end of simde-features.h, after the SIMDE_NATURAL_VECTOR_SIZE chain, so the chain's LoongArch branch never matched and the value fell through to the default of 0. Define them before that chain, next to the other architectures, and report 256 for LASX: LASX is a dual-width superset of LSX, and GCC emits a single LASX instruction for 256-bit integer, float and double vectors alike. Both changes alter which branch the existing implementations reach. The LSX/LASX paths sit after the generic paths in many #if chains and were only reachable while the value was 0, so guard those conditions to keep an explicit implementation first. Conversely the SIMDE_NATURAL_*_VECTOR_SIZE_LE(128) branches that split 256-bit intrinsics into 128-bit halves stay enabled on LoongArch, since for mm256_alignr_epi8, mm256_sad_epu8, mm256_cmov_si256 and mm256_storeu_pd the halves still reuse hand-written LSX implementations.
mr-c
reviewed
Oct 15, 2026
The LSX implementation tested imm8 bit 0 before the two-bit value, so imm8 == 3 matched the first branch and applied the 0b1001 shuffle, making it behave like imm8 == 1. Dispatch on (imm8 & 0x03) instead; the four vshuf4i_d immediates were already correct. Only imm8 == 3 was affected, and the sole test case passes imm8 == 0, which is why the bug went unnoticed. Cover the other three immediates too; with the old dispatch the imm8 == 3 case fails.
simde_mm_blend_epi32 (LSX): imm8 was used bare as an operand of >>, so the (imm8) & 0x0F handed down by the mm256 split became "13 & 0x0F >> 1", which parses as "13 & (15 >> 1)" and selects the wrong lane. Parenthesise it. simde_x_mm_negate_pd (LSX): used __lsx_vneg_d, an integer two's-complement negate, on doubles. That rewrites the exponent and mantissa as well, so 4.186863 came back as -0.976642. Flip the sign bit, as simde_x_mm256_negate_pd already does. Both branches are only reached when SIMDE_NATURAL_VECTOR_SIZE is non-zero and no native implementation applies, so neither had ever been executed.
The helper had no test at all, which is how the LSX branch came to use an integer two's-complement negate. The vectors mirror test_simde_x_mm_negate_ps, with the sign of zero compared as a bit pattern: a tolerance-based comparison cannot tell -0.0 from +0.0, and that is exactly how the sibling implementation's "0.0f - x" substitute stayed undetected.
The LSX branch computed 0.0f - x. That is a subtraction, not a sign flip: 0.0f - (+0.0f) is +0.0f, where a negate has to produce -0.0f. Flip bit 31 of each of the four elements instead. The test compared with a 0.1 tolerance, which cannot tell -0.0 from +0.0, so it now compares exactly and checks the two zeros as bit patterns. The old implementation fails that check, the new one passes it.
The LASX branch complemented the wrong comparison: NLT used cult and NLE used cule, so both came out false for unordered operands where they have to be true, and GE used clt, which comes out true for them where it has to be false. NLT therefore behaved like GE, NLE like GT, and GE like "unordered or greater than". Use clt and cle for the two negated predicates and cult for GE, which is what the simde_mm256_cmp_ps branch a few lines below does. simde_mm512_cmp_pd_mask only builds on simde_mm256_cmp_pd when SIMDE_NATURAL_VECTOR_SIZE is 256, so this stayed hidden on LoongArch for as long as the natural vector size was reported as 0. Add NaN vectors to test_simde_mm256_cmp_pd, whose inputs were all finite and so could not tell the ordered and unordered predicates apart.
…mm_cmp_ss Both LSX branches complemented the wrong comparison, the same way simde_mm256_cmp_pd did: NLT used cult and NLE used cule, which are false for unordered operands where the negated predicates have to be true, and GE used clt, which is true for them where GE has to be false. Use clt and cle for the negated predicates and cult for GE. simde_mm_cmp_ss also selected its "always false" and "always true" cases with SIMDE_LOONGARCH_LASX_NATIVE and wrote a_.i32[0] there, so those two cases went through a different union member than every other case. When the function is not inlined (SIMDE_CONSTRAINED_COMPILATION at -O1 and above) GCC then dropped the upper lanes, leaving _mm_cmp_ss with imm8 11, 15, 27 or 31 with garbage above the low element. Route them through a_.lsx_i64 like the rest. Extend test_simde_mm_cmp_sd and test_simde_mm_cmp_ss with NaN inputs, which are the only ones that tell the ordered and unordered predicates apart.
jinboson
force-pushed
the
LoongArch-fixes
branch
from
October 16, 2026 01:22
eaa3858 to
5c5534d
Compare
Collaborator
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.