Skip to content

Always use load_inst to load bytecode.#7

Merged
vext01 merged 1 commit intoykjit:masterfrom
ltratt:more_load_inst
Mar 17, 2026
Merged

Always use load_inst to load bytecode.#7
vext01 merged 1 commit intoykjit:masterfrom
ltratt:more_load_inst

Conversation

@ltratt
Copy link

@ltratt ltratt commented Mar 16, 2026

micropython uses *ip and ip[n] and friends to load bytecode directly, such that yk doesn't know those loads are idempotent. This commit fixes that, and hugely reduce the length of traces (by 3x on mandelbrot).

// bytecode version tag (see yklua for an example).
__attribute__((yk_idempotent))
byte load_inst(const byte *pc) {
NOOPT_VAL(pc);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it right to NOOPT_VAL the pc? We don't do this in yklua.

There we NOOPT_VAL the function proto version -- something we have no analog of in ykmicropython.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the NOOPT_VAL it seems that load_inst gets inlined? Maybe you can have a look and see if I'm going mad, but that's what I think has happened.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm. All I can say is that without NOOPT_VAL llvm is able to "somehow remove" load_inst(), but it's not obviously via inlining. It's kind of hard to see.

I'm going to merge this for now, as I think we have bigger fish to fry.

Maybe we can revisit this later.

@vext01 vext01 added this pull request to the merge queue Mar 16, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 16, 2026
micropython uses `*ip` and `ip[n]` and friends to load bytecode
directly, such that yk doesn't know those loads are idempotent. This
commit fixes that, and hugely reduce the length of traces (by 3x on
mandelbrot).
@ltratt
Copy link
Author

ltratt commented Mar 16, 2026

Force pushed a fix that should allow this to work when compiled in "not yk" mode.

@vext01 vext01 added this pull request to the merge queue Mar 17, 2026
Merged via the queue into ykjit:master with commit 58cf196 Mar 17, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants