-
Notifications
You must be signed in to change notification settings - Fork 27
Apply delta_ip and delta_stp from side table #690
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
Conversation
aacd79a
to
5498e06
Compare
The entire test suite is passed, but when |
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.
Thanks! I didn't review everything in details but I think the global idea is there.
Forgot to ask: How can I enable Another approach I tried: I converted it to |
You can apply the following patch: diff --git a/scripts/hwci.sh b/scripts/hwci.sh
index 4c222fd0..1f4ac986 100755
--- a/scripts/hwci.sh
+++ b/scripts/hwci.sh
@@ -77,7 +77,7 @@ full() {
}
case $1 in
- host) full unix host --arg=--protocol=unix ;;
+ host) full unix host --arg=--protocol=unix --features=wasefire-interpreter/debug ;;
# P1.01, P1.02, and P1.03 must be connected together (gpio_test).
nordic) full usb nordic ;;
*) run --protocol=${1:-usb} "$2" ;; And then simply run |
@ia0 I was wondering how to get around the following "cannot borrow In
where |
Yes, that's a common problem in Rust. You'll need to inline the definition of |
8a8240f
to
b440bf9
Compare
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.
Thanks! It looks pretty good.
I'd like to refactor incrementally. After storing |
lifetime errors
ca40438
to
6be8622
Compare
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.
Looks good to merge modulo the nits.
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, let's merge it and continue in follow-up PRs.
On my Linux Docker container: 1. CoreMark results of this PR: 37.991516 (in 18.695s), 36.7287 (in 19.258s) CoreMark results of #690: 37.101162 (in 19.133s), 37.39716 (in 18.996s) Look very similar. Not sure about `nordic`. 2. CoreMark results of `main`: 28.791477 (in 18.09s), 28.814291 (in 17.734s) So there does seem some minor performance improvement with `delta_ip` and `delta_stp`. #46 --------- Co-authored-by: Zhou Fang <33002388+zhou-w-fang@users.noreply.github.com>
Apply
delta_ip
anddelta_stp
from side table inexec.rs
.#46