-
Notifications
You must be signed in to change notification settings - Fork 77
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
interpreter: (riscv) add fmv.d implementation #2571
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2571 +/- ##
=======================================
Coverage ? 89.65%
=======================================
Files ? 357
Lines ? 45173
Branches ? 6791
=======================================
Hits ? 40498
Misses ? 3655
Partials ? 1020 ☔ View full report in Codecov by Sentry. |
Not an expert, but my understanding of this instruction is that it will cast from a float encoded as an int to a float. Is this properly handled here? (I don't know the interpreter infra) |
This just moves a float to a float, the int casting op is fcvt.w.s. |
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.
Tiny nit comment, otherwise neat stuff! 🐎
args: tuple[Any, ...], | ||
): | ||
args = RiscvFunctions.get_reg_values(interpreter, op.operands, args) | ||
results = args |
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 you could get away without this line?
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.
Defo, I'm just trying to make it explicit what's happening here, I feel like it makes it a bit clearer that it's on purpose. Maybe it could be resolved better with a comment...
No description provided.