-
Notifications
You must be signed in to change notification settings - Fork 716
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
[SIMD] add Relaxed simd #3311
base: master
Are you sure you want to change the base?
[SIMD] add Relaxed simd #3311
Conversation
Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3311 +/- ##
==========================================
- Coverage 79.83% 79.18% -0.66%
==========================================
Files 253 253
Lines 34948 35240 +292
Branches 6122 6151 +29
==========================================
+ Hits 27902 27905 +3
- Misses 5622 5904 +282
- Partials 1424 1431 +7 ☔ View full report in Codecov by Sentry. |
2c317e4
to
6ed9893
Compare
0a2c111
to
731a8c0
Compare
9d6d29c
to
aecdcf6
Compare
@q82419 Hello, may you help me review the code? And here is the relaxed simd testsuits form Webassembly. I have tested and passed and It's can put into the core testsuits. But one case |
I'll review in these days. Thanks! |
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.
Please modify for the following changes:
- Rebase on to the master. The
RelaxSIMD
proposal is added into the enum. - Filter the instructions in the
Configure
class. This will print log to ask developers to turn on this proposal. - Add the
relaxed-simd
spec test. I've updated the spec test in thewasm-dev
branch, includes the test cannot be generated bywabt
you mentioned. Please change to that branch.
Thanks!
include/executor/executor.h
Outdated
|
||
/// ======= Relaxed SIMD instructions ======= | ||
template <typename T> | ||
Expect<void> runVectorRelaxedLaneselect(ValVariant &Val1, |
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.
runVectorRelaxedLaneselectOp
lib/executor/engine/engine.cpp
Outdated
case OpCode::I64x2__relaxed_laneselect: { | ||
const ValVariant Mask = StackMgr.pop(); | ||
const ValVariant Val2 = StackMgr.pop(); | ||
return runVectorRelaxedLaneselect<uint8_t>(StackMgr.getTop(), Val2, Mask); |
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.
Should this be uint64_t
?
include/executor/executor.h
Outdated
const ValVariant &Val2, | ||
const ValVariant &Mask) const; | ||
inline Expect<void> | ||
runVectorRelaxedIntegerDotProduct(ValVariant &Val1, |
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.
runVectorRelaxedIntegerDotProductOp
include/executor/executor.h
Outdated
runVectorRelaxedIntegerDotProduct(ValVariant &Val1, | ||
const ValVariant &Val2) const; | ||
inline Expect<void> | ||
runVectorRelaxedIntegerDotProductAdd(ValVariant &Val1, const ValVariant &Val2, |
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.
runVectorRelaxedIntegerDotProductAddOp
Signed-off-by: Sylveon <[email protected]>
Signed-off-by: Sylveon <[email protected]>
Signed-off-by: Sylveon <[email protected]>
Signed-off-by: Sylveon <[email protected]>
Signed-off-by: Sylveon <[email protected]>
#3282