-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[Arm64] Add Simd HWIntrinsic test #16008
[Arm64] Add Simd HWIntrinsic test #16008
Conversation
1a95038
to
ea33e7e
Compare
@tannergooding I was wondering about you opinion about this versus #15771. In some ways it is similar, but perhaps it is a slightly different problem. I am not sure what advantage a script might offer or whether a common script could be used to generate tests for all platforms. |
{ | ||
if (!e.Message.Equals("Type is not supported.")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't take localization into account, not sure if that's an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was aware of that. For initial testing it was sufficient to prove the message was set correctly.
I'll drop it after #16102 merges.
The benefit of using a script/templates for generation is that it lets us easily add new scenarios "en masse". That is, many of the intrinsics take the same number of inputs and the only thing that differs is:
For example, The script and template means that I write the code once, run the script, and get accurate tests for all 4. I can then add any new scenarios to the template, run the script again, and have all 4 tests updated (and this actually scales much farther than just 4). |
It should definitely be possible to share the generation scripts/templates between the architectures. |
@tannergooding Would it be useful to Merge these tests? I could move the test body inside a #if ARM64_SIMD_API_PENDING_APPROVAL_AND_COREFX_MERGE
#endif |
3aafeab
to
b31ef64
Compare
I marked the individual tests with @CarolEidt @tannergooding
|
@sdmaclea, I'll give a review pass sometime later today when I get some time. I will defer to @CarolEidt on whether we should merge these before or after API review/approval |
f05d9a4
to
45b59e4
Compare
@CarolEidt Can you comment on the strategy of reviewing and merging these tests while they are disabled. It would allow the tests to be in the master branch to enable some collaboration. |
I think it's a reasonable thing to do. |
I've taken a look at this, and it seems fine to me. I didn't scrutinize it deeply, as I hope that we will replace this with a shared template between architectures. |
32f65a0
to
128687b
Compare
@CarolEidt lets merge this now that it is passing. |
No description provided.