Lower boolean conditions directly (skip boxing) + thumb fast paths - #11336
Lower boolean conditions directly (skip boxing) + thumb fast paths#11336Eric Anderson (humanapp) wants to merge 8 commits into
Conversation
Richard Knoll (riknoll)
left a comment
Richard Knoll (riknoll)
left a comment
There was a problem hiding this comment.
this change looks good (thanks for all the comments!) but agree the blast radius is high. i wish we had a test suite for generated ARM code! we'll just have to bang on it a lot.
Abhijith Chatra (@abchatra) not sure if you want this in the mbit release or not, could be risky.
|
Abhijith Chatra (@abchatra) - where are we in the testing for next release of microbit - do we have enough runway to test this change more fully? |
|
Eric Anderson (@humanapp) - the fromBool function is provided just as a helper for the user; it is not needed for the optimization, correct? |
There was a problem hiding this comment.
Pull request overview
This PR optimizes boolean handling in the PXT compiler by lowering boolean conditions directly into short-circuiting control flow (avoiding boxed booleans in branch-decision paths) and by adding Thumb inline fast paths for common boolean boxing/unboxing and truthiness checks.
Changes:
- Update
emitCondition()to lower&&/||used as conditions into short-circuiting jumps that produce raw0/1, and lower!exprin condition context viaBoolean_::bangon the raw condition result. - Add Thumb assembly helpers for
pxt::fromBool,Boolean_::bang,numops::toBool, andnumops::toBoolDecr, and route calls to these helpers where applicable.
Show a summary per file
| File | Description |
|---|---|
| pxtcompiler/emitter/emitter.ts | Lowers &&/` |
| pxtcompiler/emitter/backthumb.ts | Adds Thumb fast paths for boolean helpers (fromBool, bang, toBool, toBoolDecr) and updates comparison helper path to use the new toBoolDecr fast path. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0
|
Good to see you Eric Anderson (@humanapp) 😃 |
Thomas Ball (@thomasjball), the presence of "pxt::fromBool" in the |
|
Eric Anderson (@humanapp) - I don't quite follow the logic here:
|
|
I will create a MakeCode program to test the various conditions. |
This code is checking the lowest bit of
|
|
Eric Anderson (@humanapp) - so my confusion is how the sequence does two checks (it is a tagged int and that int is non zero). |
The cmp r0, #1 ; integer 0 ((0<<1)|1) -> false
beq .falseWith what possibility filtered out, |
|
Thanks. I chatted with Jonny Austin - we will keep this PR for after new version of MakeCode for mbit ships. |
tests/thumb-test compiles programs to native ARM Thumb in-process against a captured micro:bit (mbcodal) CompileOptions fixture and asserts on the generated assembly listing; runs offline via gulp testthumb. lang-test0 gains three semantic files covering condition truthiness and lowering and interface dispatch (including both sides of the call-site-count thresholds that gate dispatch specialization), plus a deliberately disabled repro of the default-parameters-on-dynamic-dispatch defect and a coverage map (README-codegen.md) tying failure modes to tests and layers.
Cross-platform node scripts (macOS/Linux/Windows) to build candidate and reference hexes, flash a micro:bit, capture serial, and diff the traces, with a gcStats-based soak mode for leak detection. Driven by one wrapper: npm run hwab -- ab truthiness.
Resolve backthumb.ts inlineArithmetic conflict by keeping both sides: upstream pxt::switch_eq plus the boolean fast-path entries.
Findings from running the capture against a real micro:bit: - DAPLink buffers serial while no host is reading, so a fresh capture can open onto the previously flashed program's output, including its PASS banner. Verdicts are now read only from the flashed program's own HWAB START banner onward, and --expect <case> (passed by hwab) requires the banner and verdict to name the flashed case. Timeout diagnostics report ignored foreign output and point at FAIL.TXT when no banner appears. - Panic does not flush the serial FIFO, which truncated the assert id on the wire. The generated device assert now drains for 250 ms between logging the id and panicking.
- add `noBoolLower` compile switch disabling the boolean condition lowering optimizations; with it set, output matches the pre-lowering codegen and no unused helper text is emitted - fix a template-literal syntax error in asm that broke the branch build - flip the boolbaseline expectations to the lowered codegen - add switch-variant compiles to the thumb harness; a `noBoolLower` variant pins the codegen the switch restores - verified: testlang 57, testthumb 7, A/B hexes differ under the switch; hardware A/B on micro:bit V2 -- **all three test cases pass** on both builds with identical serial traces
|
Commit 425aba7 integrates this branch with the new codegen test framework (#11561) to validate this optimization end to end. Result: all tests pass on hardware -- optimized vs reference builds of the full test corpus, A/B on a micro:bit V2, identical serial traces. Details:
|
|
Eric Anderson (@humanapp) thanks so much. Let me review this week. Bit tied with bunch of stuff. Will get back to you in a day or so. |
Hey folks! Here is another compiler optimization for your consideration. This is a change that makes every MakeCode program smaller and faster. It reduces the MicroCode hex file size by 44k, and makes the user experience on hardware noticeably more responsive. It does come with some risk: low odds of a bug, but it touches low-level branch code everywhere, so you'll want to test it broadly before shipping.
Summary
The MakeCode compiler currently compiles booleans inefficiently. In an expression like
if (!ready)orwhile (a || b), the expression is turned into a "boxed" boolean value that is then passed to a helper function, where it is unboxed and tested for truthiness. This PR changes the compiler to evaluate those conditions directly, and adds small fast-paths that speed up boxing/unboxing for a few common cases where boxing is still needed. The result is generated code that is both significantly smaller and measurably faster.Testing with the microcode build, this change produces a hex file that is ~44k smaller, and a user experience that is noticeably more responsive.
What it changes
There are two layers to this change:
!xin a condition now negatesx's truth test directly, instead of producing a boxed boolean just to invert it.a && b/a || bin a condition now compile to ordinary short-circuit branches that yield a plain0/1, instead of boxing each intermediate boolean and converting it back.Only code used to decide a branch is impacted by this change. Boolean expressions used as values (e.g.
const flag = a && b,return !x) are untouched.Risk assessment
I'd put this at low/moderate risk. The changes are small and not complicated, but the blast radius if a bug does surface is broad. In a way, the broad blast radius is risk-mitigating. If this regresses something, it would be caught pretty quickly.
The biggest risk here is misclassification of a candidate for fast-path. Sending a value that should have been boxed down the fast path would lead to incorrect truthiness evals.
Where do these classifications happen?
Fast-path filtering happens in
_numops_toBooland_numops_toBoolDecr. These methods are extensively commented to make their logic clear.Mitigating factors
Tests Performed
I compiled and ran many Arcade games on hardware, including:
cc: Thomas Ball (@thomasjball)