Skip to content

Commit 1eb6af3

Browse files
authored
Lower boolean conditions directly (skip boxing) + thumb fast paths (microsoft#11336)
* Lower boolean conditions directly (skip boxing) + thumb fast paths * Add comments to asm helpers * Add codegen test corpus and native thumb test harness 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. * Add hw-ab: hardware A/B comparison tooling for micro:bit 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. * Harden hw-ab serial capture against stale and cross-case output 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` switch and differential codegen tests - 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 --------- Co-authored-by: Eric Anderson <humanapp@users.noreply.github.com>
1 parent f37ec2d commit 1eb6af3

5 files changed

Lines changed: 263 additions & 23 deletions

File tree

‎localtypings/pxtarget.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,6 +801,7 @@ declare namespace ts.pxtc {
801801
rawELF?: boolean;
802802
multiVariant?: boolean;
803803
size?: boolean;
804+
noBoolLower?: boolean; // disable boolean condition lowering optimizations
804805
}
805806

806807
interface CompileTarget {

‎pxtcompiler/emitter/backthumb.ts‎

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,19 @@ namespace ts.pxtc {
2828
"pxt::switch_eq": "_pxt_switch_eq",
2929
}
3030

31+
// Boolean conversion fast paths, kept out of inlineArithmetic so the
32+
// `noBoolLower` flag can drop the group (redirects and helper bodies alike).
33+
const inlineBoolOps: pxt.Map<string> = {
34+
"pxt::fromBool": "_pxt_fromBool",
35+
"numops::toBool": "_numops_toBool",
36+
"numops::toBoolDecr": "_numops_toBoolDecr",
37+
"Boolean_::bang": "_pxt_boolean_bang",
38+
}
39+
40+
function boolLowerEnabled() {
41+
return !target.switches.noBoolLower
42+
}
43+
3144
// snippets for ARM Thumb assembly
3245
export class ThumbSnippets extends AssemblerSnippets {
3346
stackAligned() {
@@ -135,6 +148,8 @@ ${lbl}:`
135148
}
136149
call_lbl(lbl: string, saveStack?: boolean, stackAlign?: number) {
137150
let o = U.lookup(inlineArithmetic, lbl)
151+
if (!o && boolLowerEnabled())
152+
o = U.lookup(inlineBoolOps, lbl)
138153
if (o) {
139154
lbl = o
140155
saveStack = false
@@ -326,6 +341,100 @@ _numops_fromInt:
326341
${this.callCPPPush("pxt::fromInt")}
327342
`
328343

344+
// skipped under `noBoolLower` flag: unreferenced helper text still costs image space
345+
if (boolLowerEnabled())
346+
r += `
347+
; Tag a raw 0/1 truth value into a boolean object (taggedTrue/taggedFalse).
348+
; Inverse of the toBool fast paths below; matches C++ pxt::fromBool.
349+
_pxt_fromBool:
350+
@scope _pxt_fromBool
351+
cmp r0, #0
352+
beq .false
353+
.true:
354+
movs r0, #${taggedTrue}
355+
blx lr
356+
.false:
357+
movs r0, #${taggedFalse}
358+
blx lr
359+
360+
; Logical NOT of a raw 0/1 truth value -> raw 0/1 (matches C++ bool bang(bool)).
361+
; Callers pass an already-tested condition, so this is raw-in / raw-out; value
362+
; context (x = !y) wraps the result with fromBool to re-tag it.
363+
_pxt_boolean_bang:
364+
@scope _pxt_boolean_bang
365+
cmp r0, #0
366+
beq .true
367+
.false:
368+
movs r0, #0
369+
blx lr
370+
.true:
371+
movs r0, #1
372+
blx lr
373+
374+
; Fast-path truthiness test: tagged value in r0 -> raw 0/1 in r0.
375+
; Value encodings (see taggedSpecialValue in emitter.ts): tagged int = (n<<1)|1
376+
; (low bit set); special tags = (n<<2)|2 with undefined=0, null, false, NaN,
377+
; true; heap objects have low 2 bits 00.
378+
; Only non-pointer values are decided inline here; any heap object is handed to
379+
; the C++ helper at .boxed (which also handles boxed numbers/strings like 0.0
380+
; and ""). INVARIANT: the falsy set below must match C++ numops::toBool exactly
381+
; -- if a new falsy tagged value is ever added, it must be added here too, or it
382+
; will fall through to .true.
383+
_numops_toBool:
384+
@scope _numops_toBool
385+
cmp r0, #0 ; undefined (0) -> false
386+
beq .false
387+
cmp r0, #1 ; integer 0 ((0<<1)|1) -> false
388+
beq .false
389+
lsls r1, r0, #31 ; odd => nonzero tagged int -> true
390+
bne .true
391+
cmp r0, #${taggedNull} ; null -> false
392+
beq .false
393+
cmp r0, #${taggedFalse} ; false -> false
394+
beq .false
395+
cmp r0, #${taggedNaN} ; NaN -> false
396+
beq .false
397+
lsls r1, r0, #30 ; low 2 bits 00 => heap object -> defer to C++
398+
beq .boxed
399+
.true: ; remaining specials (e.g. true) are truthy
400+
movs r0, #1
401+
blx lr
402+
.false:
403+
movs r0, #0
404+
blx lr
405+
.boxed:
406+
${this.callCPPPush("numops::toBool")}
407+
408+
; Same truthiness classification as _numops_toBool (see comments above), but
409+
; this variant also consumes (ref-decrements) its argument. Only heap objects
410+
; are ref-counted, and those go through the C++ fallback which does the decr;
411+
; the inline non-pointer paths carry no refcount, so they need none.
412+
_numops_toBoolDecr:
413+
@scope _numops_toBoolDecr
414+
cmp r0, #0 ; undefined (0) -> false
415+
beq .false
416+
cmp r0, #1 ; integer 0 ((0<<1)|1) -> false
417+
beq .false
418+
lsls r1, r0, #31 ; odd => nonzero tagged int -> true
419+
bne .true
420+
cmp r0, #${taggedNull} ; null -> false
421+
beq .false
422+
cmp r0, #${taggedFalse} ; false -> false
423+
beq .false
424+
cmp r0, #${taggedNaN} ; NaN -> false
425+
beq .false
426+
lsls r1, r0, #30 ; low 2 bits 00 => heap object -> defer to C++
427+
beq .boxed
428+
.true:
429+
movs r0, #1
430+
blx lr
431+
.false:
432+
movs r0, #0
433+
blx lr
434+
.boxed:
435+
${this.callCPPPush("numops::toBoolDecr")}
436+
`
437+
329438

330439
for (let op of Object.keys(thumbCmpMap)) {
331440
op = op.replace(/.*::/, "")
@@ -350,9 +459,10 @@ _cmp_${op}:
350459
// the cmp isn't really needed, given how toBoolDecr() is compiled,
351460
// but better not rely on it
352461
// Also, cmp isn't needed when ref-counting (it ends with movs r0, r4)
462+
const toBoolDecr = boolLowerEnabled() ? "_numops_toBoolDecr" : "numops::toBoolDecr"
353463
r += boxedOp(`
354464
bl numops::${op}
355-
bl numops::toBoolDecr
465+
bl ${toBoolDecr}
356466
cmp r0, #0`)
357467
}
358468

‎pxtcompiler/emitter/emitter.ts‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4165,7 +4165,43 @@ ${lbl}: .short 0xffff
41654165
function emitExpressionStatement(node: ExpressionStatement) {
41664166
emitExprAsStmt(node.expression)
41674167
}
4168-
function emitCondition(expr: Expression, inner: ir.Expr = null) {
4168+
function emitCondition(expr: Expression, inner: ir.Expr = null): ir.Expr {
4169+
// with `noBoolLower` flag, the generic paths below handle every condition
4170+
const boolLower = !target.switches.noBoolLower
4171+
// Lower `&&` / `||` used as a condition into short-circuiting jumps
4172+
// that yield 0/1 directly, instead of boxing the intermediate
4173+
// booleans and converting them back with a runtime helper.
4174+
if (boolLower && !inner && !isStackMachine() && expr.kind == SK.BinaryExpression) {
4175+
const binary = expr as BinaryExpression
4176+
const op = binary.operatorToken.kind
4177+
if (op == SK.AmpersandAmpersandToken || op == SK.BarBarToken) {
4178+
const shortCircuitLabel = proc.mkLabel("lazycond")
4179+
const doneLabel = proc.mkLabel("lazycondfin")
4180+
if (op == SK.AmpersandAmpersandToken) {
4181+
proc.emitJmp(shortCircuitLabel, emitCondition(binary.left), ir.JmpMode.IfZero)
4182+
proc.emitJmp(shortCircuitLabel, emitCondition(binary.right), ir.JmpMode.IfZero)
4183+
proc.emitJmp(doneLabel, ir.numlit(1), ir.JmpMode.Always)
4184+
proc.emitLbl(shortCircuitLabel)
4185+
proc.emitJmp(doneLabel, ir.numlit(0), ir.JmpMode.Always)
4186+
} else {
4187+
proc.emitJmp(shortCircuitLabel, emitCondition(binary.left), ir.JmpMode.IfNotZero)
4188+
proc.emitJmp(shortCircuitLabel, emitCondition(binary.right), ir.JmpMode.IfNotZero)
4189+
proc.emitJmp(doneLabel, ir.numlit(0), ir.JmpMode.Always)
4190+
proc.emitLbl(shortCircuitLabel)
4191+
proc.emitJmp(doneLabel, ir.numlit(1), ir.JmpMode.Always)
4192+
}
4193+
proc.emitLbl(doneLabel)
4194+
return captureJmpValue()
4195+
}
4196+
}
4197+
// Lower `!expr` used as a condition by negating the operand's
4198+
// condition directly, avoiding a boxed boolean just to invert it.
4199+
if (boolLower && !inner && !isStackMachine() && expr.kind == SK.PrefixUnaryExpression) {
4200+
const unary = expr as PrefixUnaryExpression
4201+
if (unary.operator == SK.ExclamationToken) {
4202+
return ir.rtcall("Boolean_::bang", [emitCondition(unary.operand)])
4203+
}
4204+
}
41694205
if (!inner && isThumb() && expr.kind == SK.BinaryExpression) {
41704206
let be = expr as BinaryExpression
41714207
let mapped = U.lookup(thumbCmpMap, simpleInstruction(be, be.operatorToken.kind))

‎tests/thumb-test/asmchecks.ts‎

Lines changed: 86 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,22 @@ export function assertAtLeast(asm: string, re: RegExp, min: number, what: string
6868
chai.assert(n >= min, "expected at least " + min + " " + what + ", found " + n);
6969
}
7070

71+
/** Fails unless `re` matches the listing at most `max` times. */
72+
export function assertAtMost(asm: string, re: RegExp, max: number, what: string) {
73+
const n = countMatches(asm, re);
74+
chai.assert(n <= max, "expected at most " + max + " " + what + ", found " + n);
75+
}
76+
77+
/**
78+
* Listing up to _code_end: the program's own code, without the appended
79+
* runtime helper text (whose bodies make their own C++ fallback calls).
80+
*/
81+
export function userCode(asm: string): string {
82+
const i = asm.search(/^_code_end:/m);
83+
chai.assert(i >= 0, "no _code_end marker in listing");
84+
return asm.substr(0, i);
85+
}
86+
7187
/**
7288
* Fails unless `actual` is within `pct` percent of `expected`. Bands are wide
7389
* on purpose: they catch silent large regressions in either direction without
@@ -87,30 +103,42 @@ function escapeRegExp(s: string): string {
87103

88104
// --- per-case expectations -----------------------------------------------
89105
//
90-
// The *baseline cases pin how the compiler in this tree lowers specific
91-
// constructs, partly by asserting which specialized helpers are ABSENT from
92-
// the listing. They are the reference half of a differential pair: work that
93-
// specializes those constructs flips or extends these expectations alongside
94-
// its codegen change, and a mismatch in either direction is the signal (the
95-
// specialization silently stopped firing, or it appeared where it was not
96-
// expected). The helper names and label patterns asserted absent here are
97-
// therefore a naming contract with that work -- renaming an emitted helper or
98-
// thunk label means updating the patterns below in the same change.
106+
// The *baseline cases pin the emitted codegen in both directions: which
107+
// helpers the listing must contain and which it must not. Where an opt-out
108+
// compile switch exists, a `variantChecks` entry below compiles the same case
109+
// with the switch set and pins the codegen it restores; a mismatch in either
110+
// half is the signal. Helper and label names here are a naming contract with
111+
// the codegen -- renaming an emitted label means updating these patterns in
112+
// the same change.
99113

100114
export const asmChecks: pxt.Map<AsmCheck> = {
101115

102116
"boolbaseline.ts": (asm) => {
103-
// Conditions are materialized as tagged values and narrowed by a call
104-
// into the runtime at each test site.
105-
assertAtLeast(asm, /bl numops::toBoolDecr/g, 12, "calls to numops::toBoolDecr");
117+
// Boolean condition lowering emits thumb fast paths for the boolean
118+
// conversion runtime calls.
119+
for (const helper of ["_numops_toBool", "_numops_toBoolDecr",
120+
"_pxt_fromBool", "_pxt_boolean_bang"])
121+
chai.assert(hasLabel(asm, helper), "no " + helper + " helper in listing");
106122

107-
// Helpers introduced by boolean condition lowering. Absent here.
108-
assertAbsent(asm, [
109-
"_numops_toBool",
110-
"_numops_toBoolDecr",
111-
"_pxt_fromBool",
112-
"_pxt_boolean_bang",
113-
]);
123+
const code = userCode(asm);
124+
125+
// Every condition site narrows through the fast path; nothing in the
126+
// program still calls the C++ entry point directly.
127+
assertAtLeast(code, /bl _numops_toBoolDecr/g, 8, "calls to _numops_toBoolDecr");
128+
assertNoMatch(code, /bl numops::toBoolDecr/g, "calls to numops::toBoolDecr");
129+
130+
// `&&` / `||` in condition position short-circuit to a raw 0/1 instead
131+
// of building a tagged boolean. numops::toBool is only reached from
132+
// that boxed path, so neither entry point is called at all.
133+
assertNoMatch(code, /bl (numops::toBool|_numops_toBool)\b/g,
134+
"calls to a toBool entry point");
135+
136+
// `!` in condition position is raw-in / raw-out, so its result is no
137+
// longer re-tagged. The one remaining fromBool is the single `!` this
138+
// case uses in value position (an argument), which must still box.
139+
assertAtLeast(code, /bl _pxt_boolean_bang/g, 5, "calls to _pxt_boolean_bang");
140+
assertAtMost(code, /bl (pxt::fromBool|_pxt_fromBool)\b/g, 1,
141+
"calls to a fromBool entry point");
114142
},
115143

116144
"ifacebaseline.ts": (asm) => {
@@ -156,6 +184,45 @@ export const asmChecks: pxt.Map<AsmCheck> = {
156184
},
157185
};
158186

187+
// --- switch variants -----------------------------------------------------
188+
189+
/**
190+
* A case compiled a second time with extra compile switches and checked
191+
* against different expectations -- the opt-out half of the differential
192+
* pair described above asmChecks.
193+
*/
194+
export interface VariantCheck {
195+
/** Case file in tests/thumb-test/cases. */
196+
caseFile: string;
197+
/** Merged over the case's normal switches. */
198+
switches: pxtc.CompileSwitches;
199+
/** Names the variant in the test title; keep it short. */
200+
label: string;
201+
check: AsmCheck;
202+
}
203+
204+
export const variantChecks: VariantCheck[] = [
205+
206+
{
207+
caseFile: "boolbaseline.ts",
208+
switches: { noBoolLower: true },
209+
label: "noBoolLower",
210+
check: (asm) => {
211+
// Conditions are materialized as tagged values and narrowed by a
212+
// call into the runtime at each test site.
213+
assertAtLeast(asm, /bl numops::toBoolDecr/g, 12, "calls to numops::toBoolDecr");
214+
215+
// The thumb fast paths are neither emitted nor called.
216+
assertAbsent(asm, [
217+
"_numops_toBool",
218+
"_numops_toBoolDecr",
219+
"_pxt_fromBool",
220+
"_pxt_boolean_bang",
221+
]);
222+
},
223+
},
224+
];
225+
159226
// --- external case programs ----------------------------------------------
160227

161228
/**

‎tests/thumb-test/thumbrunner.ts‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import * as zlib from 'zlib';
88
import "mocha";
99
import * as chai from "chai";
1010

11-
import { asmChecks, externalCases } from "./asmchecks";
11+
import { asmChecks, externalCases, variantChecks } from "./asmchecks";
1212

1313
const testDir = path.join(process.cwd(), "tests", "thumb-test");
1414
const casesDir = path.join(testDir, "cases");
@@ -42,7 +42,8 @@ function loadFixtureText(): string {
4242
return zlib.gunzipSync(fs.readFileSync(fixturePath)).toString("utf8");
4343
}
4444

45-
function optionsFor(fixtureText: string, programText: string): pxtc.CompileOptions {
45+
function optionsFor(fixtureText: string, programText: string,
46+
switches?: pxtc.CompileSwitches): pxtc.CompileOptions {
4647
const opts: pxtc.CompileOptions = JSON.parse(fixtureText).options;
4748

4849
opts.fileSystem[pxt.MAIN_TS] = programText;
@@ -52,6 +53,8 @@ function optionsFor(fixtureText: string, programText: string): pxtc.CompileOptio
5253
opts.target.switches = opts.target.switches || {};
5354
// Emits the code-size stats header at the top of the listing.
5455
opts.target.switches.size = true;
56+
if (switches)
57+
Object.assign(opts.target.switches, switches);
5558

5659
return opts;
5760
}
@@ -108,6 +111,29 @@ describe("thumb codegen", () => {
108111
});
109112
});
110113

114+
// Second compile of a case with extra compile switches; see variantChecks.
115+
variantChecks.forEach(variant => {
116+
it("compiles " + variant.caseFile + " to thumb with " + variant.label, function () {
117+
this.timeout(TIMEOUT_MS);
118+
119+
const casePath = path.join(casesDir, variant.caseFile);
120+
chai.assert(fs.existsSync(casePath), "missing case " + casePath);
121+
122+
const programText = fs.readFileSync(casePath, "utf8");
123+
const opts = optionsFor(fixtureText, programText, variant.switches);
124+
125+
const res = pxtc.compile(opts);
126+
chai.assert(res.success,
127+
"native compile of " + variant.caseFile + " with " + variant.label +
128+
" failed:\n" + describeDiagnostics(res));
129+
130+
const asm = res.outfiles[pxtc.BINARY_ASM];
131+
chai.assert(!!asm, "no " + pxtc.BINARY_ASM + " in compile result");
132+
133+
variant.check(asm, res);
134+
});
135+
});
136+
111137
// Semantic case programs kept elsewhere in the tree; see externalCases.
112138
// They are compiled and assembled here, not executed.
113139
const externalPaths = Object.keys(externalCases).sort();

0 commit comments

Comments
 (0)