py/emitnative: Let emitters know the compiled entity's name. #17572
Conversation
|
Code size report: |
agatti
force-pushed
the
emitnative-provide-entity-name
branch
from
6cbc681 to
524ce75
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #17572 +/- ##
==========================================
- Coverage 98.57% 98.56% -0.01%
==========================================
Files 169 169
Lines 21968 21952 -16
==========================================
- Hits 21654 21638 -16
Misses 314 314 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This commit extends the vstr_t API with an additional function that lets inserting a null-terminated string anywhere in a vstr_t buffer. Insertion was only supported for two cases, a single byte or a single character; both of them would use an internal function that allows inserting an arbitrary number of bytes anywhere in the vstr_t buffer, but with a fixed size of 1. A new function call `vstr_ins_str` is thus added, that still uses the same mechanism described above, but with the computed string length as the inserted block size. The same parameter checking caveats from `vstr_ins_char` and `vstr_ins_byte` still apply here. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
This commit introduces an optional feature to provide to native emitters the fully qualified name of the entity they are compiling. This is achieved by altering the generic ASM API to provide a third argument to the entry function, containing the name of the entity being compiled. Currently only the debug emitter uses this feature, as it is not really useful for other emitters for the time being; in fact the macros in question just strip the name away. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
agatti
force-pushed
the
emitnative-provide-entity-name
branch
from
524ce75 to
6a7d117
Compare
| @@ -364,6 +364,11 @@ static mp_obj_t extra_coverage(void) { | |||
| } else { | |||
| mp_obj_print_exception(&mp_plat_print, MP_OBJ_FROM_PTR(nlr.ret_val)); | |||
| } | |||
|
|
|||
| vstr_ins_str(vstr, 0, "\"tests\""); | |||
There was a problem hiding this comment.
If this is just to test vstr_ins_str maybe it should clear the vstr to start with, instead of using vstr_cut_tail_bytes?
There was a problem hiding this comment.
There's no other vstr_ins_* function that's called with 0 as its insertion point in this test file. Since all the insertion functions share the same extra prefix allocation bits I thought it'd be nice to cover that specific behaviour too.
But I can let it perform an insertion on an empty vstr too, or better yet cover both cases.
| @@ -210,6 +210,12 @@ void vstr_ins_char(vstr_t *vstr, size_t char_pos, unichar chr) { | |||
| *s = chr; | |||
| } | |||
|
|
|||
| void vstr_ins_str(vstr_t *vstr, size_t str_pos, const char *str) { | |||
There was a problem hiding this comment.
It looks like this addition is increasing code size for all ports, which is unexpected (due to gc sections).
Needs investigation.
There was a problem hiding this comment.
Maybe for simplicity, just write this function inline where it's used (once)?
There was a problem hiding this comment.
To get there I'd have to make vstr_ins_blank_bytes non-static, I'll check if that would increase the code footprint or not.
Summary
This PR introduces an optional feature to provide to native emitters the fully qualified name of the entity they are compiling.
This is achieved by altering the generic ASM API to provide a third argument to the entry function, containing the name of the entity being compiled. Currently only the debug emitter uses this feature, as it is not really useful for other emitters for the time being; in fact the macros in question just strip the name away.
As discussed in #17548, this brings more descriptive output to the debug emitter, in preparation for a new machine-readable JSON output emitter that would need to know the entity name being processed as well.
Testing
Testing had to be performed manually, by checking the output of
mpy-cross -X emit=native -march=debugfor a variety of existing MicroPython test files containing all possible entities to be handled by the compiler framework along with their nested variants. The usual battery of CI tests was also ran through Github to be sure nothing out of the ordinary would happen.Trade-offs and Alternatives
In theory there shouldn't be any changes in any other emitter that actually generates binary code, this code should only affect
mpy-cross.