simde icon indicating copy to clipboard operation
simde copied to clipboard

Some functions don't have their own tests

Open mr-c opened this issue 4 years ago • 10 comments

I've added some in https://github.com/nemequ/simde/tree/missing_tests

  • [x] _mm256_castpd_ps
  • [x] _mm256_castpd_si256
  • [x] _mm256_castps128_ps256
  • [x] _mm256_castps256_ps128
  • [x] _mm256_castps_pd
  • [x] _mm256_castps_si256
  • [x] _mm256_castsi128_si256
  • [x] _mm256_castsi256_pd
  • [x] _mm256_castsi256_ps
  • [x] _mm256_castsi256_si128
  • [x] _mm256_permute2f128_pd
  • [x] _mm256_permute2f128_si256
  • [x] _mm256_undefined_pd
  • [x] _mm256_undefined_ps
  • [x] _mm256_undefined_si256
  • [x] _mm256_xor_pd
  • [x] _mm256_xor_ps
  • [x] _mm512_add_epi16
  • [x] _mm512_add_epi8
  • [x] _mm512_castpd128_pd512
  • [x] _mm512_castpd256_pd512
  • [x] _mm512_castpd_ps
  • [x] _mm512_castpd_si512
  • [x] _mm512_castps128_ps512
  • [x] _mm512_castps256_ps512
  • [x] _mm512_castps_pd
  • [x] _mm512_castps_si512
  • [x] _mm512_castsi128_si512
  • [x] _mm512_castsi256_si512
  • [x] _mm512_castsi512_pd
  • [x] _mm512_castsi512_ps
  • [x] _mm512_set1_epi16
  • [x] _mm512_set1_epi32
  • [x] _mm512_set1_epi64
  • [x] _mm512_set1_epi8
  • [x] _mm512_set1_pd
  • [x] _mm512_set1_ps
  • [x] _mm512_set4_epi32
  • [x] _mm512_set4_epi64
  • [x] _mm512_set4_pd
  • [x] _mm512_set4_ps
  • [x] _mm512_set_epi16
  • [x] _mm512_set_epi32
  • [x] _mm512_set_epi64
  • [x] _mm512_set_epi8
  • [x] _mm512_set_pd
  • [x] _mm512_set_ps
  • [x] _mm512_setr4_epi32
  • [x] _mm512_setr4_epi64
  • [x] _mm512_setr4_pd
  • [x] _mm512_setr4_ps
  • [x] _mm512_setr_epi32
  • [x] _mm512_setr_epi64
  • [x] _mm512_setr_pd
  • [x] _mm512_setr_ps
  • [x] _mm512_setzero_pd
  • [x] _mm512_setzero_ps
  • [ ] _mm_castpd_si128
  • [x] _mm_castps_pd
  • [ ] _mm_castps_si128
  • [x] _mm_set1_epi16
  • [x] _mm_set1_epi32
  • [x] _mm_set_epi16
  • [x] _mm_set_epi32
  • [x] _mm_set_epi64x
  • [x] _mm_set_epi8
  • [x] _mm_set_pd
  • [x] _mm_set_pd1
  • [ ] _mm_set_pi16
  • [ ] _mm_set_pi32
  • [ ] _mm_set_pi8
  • [x] _mm_set_ps1
  • [x] _mm_setr_epi64
  • [x] _mm_setr_pd
  • [x] _mm_setzero_pd
  • [x] _mm_setzero_si64
  • [x] _mm_store1_pd
  • [x] _mm_undefined_pd
  • [x] _mm_undefined_si128

The following are missing tests, but I believe they are unsupported / no-ops for SIMDe:

  • _mm_clflush
  • _mm_empty
  • _mm_getcsr
  • _mm_lfence
  • _mm_mfence
  • _mm_pause
  • _mm_prefetch
  • _mm_setcsr
  • _mm_sfence

_mm256_alignr_epi8 Initially showed up on my list because it is tested via multiples tests: test_simde_mm256_alignr_epi8_case0 throught test_simde_mm256_alignr_epi8_case3

mr-c avatar Apr 24 '20 14:04 mr-c

I'm working on the checked entries over at https://github.com/nemequ/simde/compare/missing_tests

mr-c avatar Apr 24 '20 14:04 mr-c

I wouldn't bother with the _mm*set* ones like _mm512_set_epi32, _mm512_set_ps, _mm512_set_pd. Those will be tested by almost every other function that operates on vector of that size and type.

nemequ avatar Apr 24 '20 16:04 nemequ

@mr-c Hello! I just wanted to confirm if you are working on _mm512_add_epi*, _mm512_castpd128_pd512, _mm512_castpd256_pd512, if not, can I start working on such functions or any others that you suggest?

himanshi18037 avatar Apr 24 '20 17:04 himanshi18037

@himanshi18037 I don't have access to AVX512 anything, so I can't generate the test arrays; if you've got it then go for it!

(Unlinke @nemequm, I think the _mm512_set* functions are good to have tests for, as completeness helps with the SIMDE_NATIVE_ALIASES testing)

mr-c avatar Apr 24 '20 17:04 mr-c

@himanshi18037 I don't have access to AVX512 anything, so I can't generate the test arrays; if you've got it then go for it!

Ok, I will try to add test cases for these functions soon.

himanshi18037 avatar Apr 24 '20 17:04 himanshi18037

I think Himanshi and I are both using Intel's Software Development Emulator for that: https://software.intel.com/en-us/articles/intel-software-development-emulator/

The set functions are a pain to test since the tests use them to generate the vectors to compare to. You really need to use compare to load/store(or loadu/storeu), but at that point you're just duplicating the tests for those functions.

nemequ avatar Apr 24 '20 17:04 nemequ

I think every test matters, I would not have expected the sort of the fixes need to make the new tests in https://github.com/nemequ/simde/pull/234/files work!

mr-c avatar Apr 24 '20 18:04 mr-c

I think every test matters, I would not have expected the sort of the fixes need to make the new tests in https://github.com/nemequ/simde/pull/234/files work!

The other tests are definitely important. I'm only talking about simde_mm{,256,512}_set_{ps,pd,epi8,epi16,epi32,epi64,pi8,pi16,pi32} (and maybe a couple others like simde_mm_). Those are tested in hundreds of other functions, so tests specific to them don't provide much benefit other than being able to check them off a list. I'm not saying I wouldn't accept a patch, especially if you want them, only that I wouldn't bother since they don't have much practical value. Unless the goal here is to get CI set up so it fails if we're missing a test, in which case adding tests to get that check to pass would be worth it.

The other functions you found without test cases are bad, and not having tests for native aliases in general is very bad; I'm really happy you're working on that.

We'll also have to be a bit creative with the cast functions, at least the ones where we're casting to elements of different types (i.e., castps128_ps512 isn't a problem, but castps_pd or castepi16)

Ideally I'd like to get CodeCov (or Coveralls, or something else) back in place, but unfortunately for the past several months it was unreliable to the point of uselessness. I wasn't able to get Coveralls working on Travis (CMake problems IIRC, not Travis' fault), but I think I'll see if I can get it working on Drone since we're using Meson there, and Meson has coverage support built in. Also, it's vastly less unpleasant to work with than CMake.

nemequ avatar Apr 24 '20 19:04 nemequ

@himanshi18037 I don't have access to AVX512 anything, so I can't generate the test arrays; if you've got it then go for it!

Hello @mr-c, I am done with almost all AVX512 functions test cases, just a few mm512_set_x functions are left which I don't know how to tackle right now, so if you are not working on the unchecked mm/mm256 functions, I can try implementing those, or any others that you suggest.

himanshi18037 avatar May 03 '20 17:05 himanshi18037

Thanks @himanshi18037 ! Anything without a checkmark is fair game, thanks!

mr-c avatar May 03 '20 17:05 mr-c