py/asmrv32: Fix RV32 extended test failures. #15575
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Marked as draft as this does not fully fix all tests yet. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #15575 +/- ##
=======================================
Coverage 98.43% 98.43%
=======================================
Files 163 163
Lines 21290 21290
=======================================
Hits 20956 20956
Misses 334 334 ☔ View full report in Codecov by Sentry. |
|
Code size report: |
dpgeorge
reviewed
Aug 1, 2024
77490d9 to
ba9d582
Compare
dpgeorge
reviewed
Aug 2, 2024
dpgeorge
reviewed
Aug 2, 2024
dpgeorge
reviewed
Aug 2, 2024
ba9d582 to
f074a1c
Compare
|
This PR now contains fixes for all RV32-specific test failures found in #15551, it's now marked as ready for review. |
f074a1c to
b56adda
Compare
b56adda to
76ecea3
Compare
|
I tested this PR on a RP2350 in RISC-V mode and it works well, all |
76ecea3 to
e649f1f
Compare
|
I've cleaned the PR up a bit, and did the same successful test verification you did but on an ESP32C3. |
The RV32 emitter always scheduled short jumps even outside the emit compiler pass. Running the full test suite through the native emitter instead of just the tests that depend on the emitter at runtime (as in, `micropython/native_*` and `micropython/viper_* tests`) uncovered more places where the invalid behaviour was still present. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
The RV32 emitter sometimes generated short load opcodes even when it was not supposed to. This commit fixes an off-by-one error in its offset eligibility range calculation and corrects one case of offset calculation, operating on the raw label index number rather than its effective offset in the stack (C.LW assumes all loads are word-aligned). Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
The RV32 emitter used an additional temporary register, as certain code sequences required extra storage. This commit removes its usage in all but one case, using REG_TEMP2 instead. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
e649f1f to
7d8b2d8
Compare
|
This looks really good now, thank you. I also reconfirmed the latest changes on RP2350-RISCV. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR contains fixes for the native emitter test failures uncovered in #15551. These fixes depend on #15573 to be merged in to properly fix test some test failures.
Testing
Ran the whole
esp32test suite viampy-crosson an ESP32C3 board with./run-tests.py --target esp32 --device /dev/ttyUSB0 --via-mpy --emit native --mpy-cross-flags="-march=rv32imc".