Hi Christophe, Thanks for the review!
> > A while ago I added p64_p128.c, to contain all the poly64/128 tests except for > vreinterpret. > Why do you need to create p64.c ? I originally created it because I had a much smaller set of intrinsics that I wanted to add initially, this grew and It hadn't occurred to me that I can use the existing file now. Another reason was the effective-target arm_crypto_ok as you mentioned below. > > Similarly, adding tests for vcreate_p64 etc... in p64.c or p64_p128.c might be > easier to maintain than adding them to vcreate.c etc with several #ifdef > conditions. Fair enough, I'll move them to p64_p128.c. > For vdup-vmod.c, why do you add the "&& defined(__aarch64__)" > condition? These intrinsics are defined in arm/arm_neon.h, right? > They are tested in p64_p128.c I should have looked for them, they weren't being tested before so I had Mistakenly assumed that they weren't available. Now I realize I just need To add the proper test option to the file to enable crypto. I'll update this as well. > Looking at your patch, it seems some tests are currently missing for arm: > vget_high_p64. I'm not sure why I missed it when I removed neont- > testgen... I'll adjust the test conditions so they run for ARM as well. > > Regarding vreinterpret_p128.c, doesn't the existing effective-target > arm_crypto_ok prevent the tests from running on aarch64? Yes they do, I was comparing the output against a clean version and hasn't noticed That they weren't running. Thanks! > > Thanks, > > Christophe
