-
Notifications
You must be signed in to change notification settings - Fork 675
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
wasm2c: include simd128.h and wasm-rt-exceptions.h where necessary #2236
Conversation
keithw
commented
May 12, 2023
- Adapts @shravanrn's wasm2c: Only add simd and exception code if features are used #2229.
- Directly include SIMDe's simd128.h and wasm-rt-exceptions.h where needed.
- Only includes simd128.h in the header file (and embedder's namespace) if the module requires it -- otherwise it only goes in the .c (or -impl.h) file.
- Would check boxes 6 and 7 in wasm2c SIMD final approach checklist #2224.
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.
lgtm % comment
} | ||
} | ||
|
||
void CWriter::WriteV128Decl() { | ||
Write("#include <simde/wasm/simd128.h>", Newline(), Newline()); | ||
Write("#ifndef WASM_RT_SIMD_TYPE_DEFINED", Newline(), |
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.
Do you need the guard given that you also have simd_used_in_header_?
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.
Unfortunately I think we still need the guard, given that this is sometimes going in the .h file, and C forbids duplicate typedefs even if they're identical. If the embedder includes two module .h files, we don't want a collision. :-/
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.
lgtm.. I'll let @shravanrn approve since its based on his PR.
[&](const auto& x) { | ||
return x->kind == ExternalKind::Func && | ||
func_uses_simd(module_->GetFunc(x->var)->decl.sig); | ||
})); |
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.
How come we have have all this nice modern style of C++ but we still have to type of out .begin()
and .end()
everywhere.. :(
Also, why does the third lambda have the &
capture specifier? (sorry I'm still not used to reading C++ lambdas).
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 think if we upgraded to C++20 we could probably use std::ranges::any_of
, which finally gets rid of the .begin() and .end().
The third lambda is the only one that uses the module_
member (for GetFunc), so it needs to capture it (and capturing implicitly with &
seemed like the simplest way to do that).