Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial Implementation for the new WASM based instruction set #952

Merged

Conversation

anutosh491
Copy link
Contributor

No description provided.

@anutosh491
Copy link
Contributor Author

anutosh491 commented Oct 9, 2023

Now if I understand correctly as WASM support for CI has been introduced, the doctests would be run in a wasm runtime (for all archs including the wasm one ?) . Hence we would need all essential function ( or rather functions that don't have a fallback implementation in our xsimd_wasm.hpp file) so that we are able to compile the code.

How do we plan to test the new instruction set out if that's the case. We could surely implement everything in this pr itself if we would like to. Not sure if we could maybe stop it here as we already have some operations implemented here and take care of other operations ( in a bunch) in subsequent prs!

include/xsimd/arch/xsimd_wasm.hpp Show resolved Hide resolved
{
assert(false && "unsupported arch/op combination");
return {};
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This funciton and the previous one have duplicated implementations, maybe a common "impl" mehtod accepting the underlying WASM registers could be used to avoid code duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I'll try refactoring any code duplication or repetition in the subsequent PR.

include/xsimd/arch/xsimd_wasm.hpp Outdated Show resolved Hide resolved
include/xsimd/arch/xsimd_wasm.hpp Outdated Show resolved Hide resolved
include/xsimd/arch/xsimd_wasm.hpp Outdated Show resolved Hide resolved
}
else XSIMD_IF_CONSTEXPR(sizeof(T) == 4)
{
return from_mask(batch_bool<float, A> {}, mask, wasm {});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we would like to convert the batch_bool<float, A> value returned here to a batch_bool<int32_t, A> but I am not sure we have an intrinsic or any operation already for it !

1) Bitwise: bitwise_rshift, bitwise_not, bitwise_and, bitwise_or, bitwise_lshift, bitwise_xor, bitwise_andnot
2) Logical: gt, lt, eq, neq, all, any, ge, le
3) Arithmetic: add, sub, mul, div, neg, reciprocal
4) Math: abs, sqrt, rsqrt, max, min
5) Roudning: floor, ceil , trunc
6) Memory: store_aligned, store_unaligned, load_aligned, load_unaligned, set
7) Complex: isnan
7) Misc: mask, select, broadcast, insert
@JohanMabille
Copy link
Member

Hey @anutosh491 what about formatting this so the CI can be green and then we can merge and iterate in the next PR? This would ease the review process.

@anutosh491
Copy link
Contributor Author

Yes that sounds like a plan. I've made a commit which should hopefully fix the style format error.

1) arithmetic: sadd, ssub, hadd, haddp
2) batch manipulation: zip_lo, zip_hi, slide_left, slide_right
3) math: reduce_add
4) memory: store_complex, load_complex
5) misc: from_mask
@anutosh491
Copy link
Contributor Author

TODO

  1. Look after casting operations from one batch/batch_bool type to another.
  2. Some operations like reduce_max, reduce_min, shuffle, swizzle, fast_cast are left to be implemented.

@JohanMabille
Copy link
Member

Do not hesitate to open an issue to track them, and check the boxes as you implement them.

@anutosh491
Copy link
Contributor Author

anutosh491 commented Oct 24, 2023

Let me know if you would like me to squash all commits into a couple/few meaningful commits with good commit messages :)

@JohanMabille
Copy link
Member

Indeed, that would nice. Thank you!

@anutosh491
Copy link
Contributor Author

I'll also get rid of the test file I had added just for reference to get started (test/test_wasm/test_wasm.cpp). Won't really need that !

@anutosh491 anutosh491 force-pushed the Introducing_wasm_based_IS branch from 3d98a7d to 1402e0e Compare October 24, 2023 15:17
@anutosh491
Copy link
Contributor Author

anutosh491 commented Oct 24, 2023

Hey @JohanMabille I've squashed the commits and hopefully they look more meaningful now. I do think we can take up all refactoring and other things up in subsequent PRs. For now if you don't find any obvious errors, this might be ready.

@JohanMabille JohanMabille merged commit f27bbb8 into xtensor-stack:master Oct 25, 2023
49 checks passed
@SylvainCorlay
Copy link
Member

Congrats @anutosh491, this is a great milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants