simde
simde copied to clipboard
Fix qdmlsl instructions
The qdmlsl instructions were implemented without any saturation. This has been fixed by utilising existing saturating instructions which are implemented correctly.
Unit tests have also been updated to test for saturation.
Looks like the high_n unit tests are incorrect, looking into it
Looks like the high_n unit tests are incorrect, looking into it
Thanks. I'm still seeing some test failures:
- [x] clang (17, ubuntu-22.04, -march=native -Wno-unsafe-buffer-usage -O2) https://github.com/simd-everywhere/simde/actions/runs/10468306869/job/28988926307?pr=1195#step:9:1023
- [x] i686-gcc11-O2 https://app.circleci.com/pipelines/github/simd-everywhere/simde/1698/workflows/8926b5aa-d8ed-4bff-9593-f67e8928897f/jobs/4801?invite=true#step-112-139088_70
- [x] emscripten https://github.com/simd-everywhere/simde/actions/runs/10468306869/job/28988911124#step:9:2037
- [x] gcc-qemu (-O3, 12, riscv64, riscv64, riscv64, ubuntu-22.04) https://github.com/simd-everywhere/simde/actions/runs/10468306869/job/28988921061?pr=1195#step:10:1020
There is a compile failure with MSVC, maybe that will be helpful: https://ci.appveyor.com/project/nemequ/simde/builds/50436859/job/4lpievni4ibfsa7h#L2440 (our entire MSVC CI matrix failed, not just that version: https://ci.appveyor.com/project/nemequ/simde/builds/50436859)
Ping @Ryo-not-rio ; any update on this or your other PR?
@Syonyk If you get a chance, your feedback here would be appreciated
I believe these patches are correct, but were out of date with head. It looks like you've pulled them forward, so they now mostly have the changes required. I'm happy to take a closer look at this once I have my other fixes pushed, and I can test the updated files in my test suite as well.
If ryo-not-rio is not responding to requests to update tests and such, I can make a separate pull request with these changes, and add the proper test vectors. These functions were quite wrong before, though.
Thank you @Syonyk ; looks like the previous compiler issues have been fixed so I've set this PR to automerge once the remaining CI checks are finished. Of course, if you still have feedback later that would be very welcome!
Ha, I spoke too soon: MSVC is unhappy: https://ci.appveyor.com/project/nemequ/simde/builds/51262044/job/hojt0jq3999w07j7#L2451
../test/arm/neon/qdmlsl_high_lane.c(76): warning C4003: not enough arguments for function-like macro invocation 'simde_vqdmlsl_high_lane_s16'
../test/arm/neon/qdmlsl_high_lane.c(76): error C2440: 'function': cannot convert from 'simde_int16x4_t' to 'simde_int32x4_t'
../test/arm/neon/qdmlsl_high_lane.c(76): warning C4024: 'simde_vqdmlsl_s16': different types for formal and actual parameter 1
../test/arm/neon/qdmlsl_high_lane.c(76): error C2440: 'function': cannot convert from 'int' to 'simde_int16x8_t'
../test/arm/neon/qdmlsl_high_lane.c(76): warning C4024: 'simde_vget_high_s16': different types for formal and actual parameter 1
../test/arm/neon/qdmlsl_high_lane.c(76): error C2059: syntax error: ')'
Those look like valid errors... I can certainly take a look, but it won't be until next week. I'm nearing the end of my stare-at-screen time for the week.
Those look like valid errors... I can certainly take a look, but it won't be until next week. I'm nearing the end of my stare-at-screen time for the week.
No worries, I got a fix