diff --git a/docs/superpowers/plans/2026-09-22-32-bit-phase-4a-union-byte-storage.md b/docs/superpowers/plans/2026-09-22-32-bit-phase-4a-union-byte-storage.md new file mode 100644 index 000000000..c935dd56a --- /dev/null +++ b/docs/superpowers/plans/2026-09-22-32-bit-phase-4a-union-byte-storage.md @@ -0,0 +1,212 @@ +# 32-bit Phase 4a: Union Byte Storage Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Every member of a tagged union reads back exactly the value it was stored with, on x64 and on i686, under every memory model. + +**Architecture:** A tagged union lowers to a packed `{ptr tag, S}`, where `S` is the LLVM type of its largest member (`findMaxSizeType`). A member goes in and out through memory: `castLLVMTypes` stores it, `CopyStructOp` copies `min(src, dst)` bytes, and the result is loaded back. When that memory is loaded as `S`, any bytes in `S`'s *padding* are not part of the loaded value. So a smaller member's field that sits in that padding is lost the next time the union is copied as a value. + +At x64 this happens only for some layouts; the old test `00bigint_struct_layout` documents one and cannot assert its field `a`. At i686 it happens for common ones, because a pointer is 4 bytes and an `f64` after it is 8-aligned. `02union_type` then prints `Error 9.11037e-305 downloading` instead of `Error 1 downloading`. + +The fix is to make `S` an `[N x i8]` array, where N is the largest member's alloc size. A byte array has no padding, so every byte is carried. The in and out paths already go through memory, so they need no new logic. Each consumer of the union's value field must be checked to still reinterpret correctly. + +**Tech Stack:** C++17, the MLIR LLVM dialect, the TypeScript dialect lowering (`LowerToLLVM.cpp`, `CastLogicHelper.h`, `OwnershipRoutineLogic.h`, `LLVMDebugInfo.h`), and bash test scripts. + +**Spec:** `docs/superpowers/specs/2026-09-20-32-bit-compilation-design.md`: the Phase 4 section, and "Open issues". The union defect was deferred in phase 2 (ruling 1). A corpus probe of phase 4 found that it breaks real tests at i686. + +## Global Constraints + +- **The x64 output changes by design:** the storage type of every tagged union becomes `[N x i8]`. So the phase 2/3 rule "x64 IR unchanged" does **not** apply to this plan. It is replaced by the following: + - `ctest -j 16 -C Release` from `__build/tslang/windows-msbuild-2026-release` stays at 2766/2766, plus any tests this plan adds. + - The corpus probe (below) shows no file that passed before and fails after, at either arch and under any memory model. + - x64 code size is measured and reported, not gated. +- **Union size and tag layout are unchanged** apart from the storage type. `N` is the same alloc size the old storage type had, so `sizeof` of every union stays the same. Check this. +- **Ownership (`-mm=rc`) must stay exact.** The rc verifier `--verify-ownership` must stay silent wherever it is silent today. +- Hard errors, not silent fallbacks. +- **Corpus probe:** `tslang/test/probe-corpus.sh`, added in Task 1. It compiles and runs every file in `tslang/test/tester/tests` as a single-file exe with the suite's flags (`--opt --opt_level=3 --no-default-lib --entry-point`), for x64 and i686 × gc, rc and none, and writes a TSV. The baseline TSV is taken from the plan's base commit. About 100 files per model fail at x64 too; those are multi-file tests that need the test runner, and "no new failures" is judged against the baseline, not against zero. +- **x86 library flags:** + - i686: `--gc-lib-path=/3rdParty/gc/x64/release/lib`, `--tslang-lib-path=/__build/tslang-runtime/release`. + - x64: `--tslang-lib-path=/__build/tslang/windows-msbuild-2026-release/lib` and the same gc path. + - Unset `GC_LIB_PATH`, `TSLANG_LIB_PATH`, `GC_SHARED_LIB_PATH` and `DEFAULT_LIB_PATH`. +- Commits are GPG-signed. Never bypass signing; if it times out, leave the change staged and report. The trailer is `Co-Authored-By: Claude Opus 5 `. +- Tests never read a catch variable's value. + +## Measured starting point (main b61e6c6c, corpus probe) + +| | x64 pass | i686 pass | +| --- | --- | --- | +| gc | 423 | 410 | +| rc | 418 | 409 | +| none | 418 | 411 | + +Fifteen files fail only at i686. Of those, `02union_type` is confirmed to be this defect, and `03union_type` and `03union_type_case_order` show the same `types have different sizes` warnings. `internals`, `00typeof_static_fold_conditions`, `parser`, `arrayLiterals` and `arrayLiterals2ES5` (rc) are not diagnosed; some may be this defect. The async files belong to phase 4b. + +## File structure + +| File | Change | Task | +| --- | --- | --- | +| `tslang/test/probe-corpus.sh` | New. Runs the whole corpus as single-file exes and writes a TSV. | 1 | +| `tslang/test/tester/tests/00union_member_padding.ts` | New. Round-trips union members whose fields fall in another member's padding. | 1 | +| `tslang/test/tester/tests/00bigint_struct_layout.ts` | Asserts `a` again. | 1 | +| `tslang/test/tester/CMakeLists.txt` | Registers the new test. | 1 | +| `tslang/lib/TypeScript/LowerToLLVM.cpp` (~6690, the `UnionType` conversion) | Storage becomes `[N x i8]`. | 2 | +| `tslang/include/TypeScript/LowerToLLVM/LLVMTypeConverterHelper.h` (`findMaxSizeType`) | Adds a storage-size query. | 2 | +| `tslang/include/TypeScript/LowerToLLVM/CastLogicHelper.h`, `OwnershipRoutineLogic.h`, `UnaryBinLogicalOrHelper.h`, `LLVMDebugInfo.h`, `LowerToLLVM.cpp` (union op lowerings ~2123-2230) | Each reader of the union value is audited and fixed where needed. | 2 | + +--- + +### Task 1: Failing tests and the corpus probe + +**Files:** +- Create: `tslang/test/probe-corpus.sh`, `tslang/test/tester/tests/00union_member_padding.ts` +- Modify: `tslang/test/tester/tests/00bigint_struct_layout.ts`, `tslang/test/tester/CMakeLists.txt` + +**Interfaces:** +- Produces: + - `tslang/test/probe-corpus.sh [jobs]`. It writes `/results.tsv` with the columns `name arch mm stage code`, where `stage` is one of `pass`, `compile`, `link`, `run` or `timeout`. + - `tslang/test/probe-corpus.sh --compare `. It prints every row that passed before and does not pass after, then a summary line. It exits non-zero if there is any such row. + +- [ ] **Step 1: The probe script.** + - One compile-and-run per file, arch (`x64`: `x86_64-pc-windows-msvc`, `x86`: `i686-pc-windows-msvc`) and memory model. + - It runs with `xargs -P ` (default 12), a 30 s `timeout` per run, and in a per-case directory under the output dir. Exes and objects are deleted after each run. + - It unsets the lib env vars and passes the lib flags from Global Constraints. It fails loudly (`FAIL missing prerequisite`) if `tslang.exe`, the libs or `timeout` are missing. + - The classification: compile exit ≠ 0 with `LNK` in the output is `link`; any other non-zero compile exit is `compile`; a run exit of 124 is `timeout`; any other non-zero run exit is `run`. + - Keep it hand-run, like the other `check-*.sh` scripts. + - Run it at this plan's base commit and keep `results.tsv` as the baseline, outside the repo, in the SDD workspace. Report the per-arch and per-model counts; they should match the table above within a file or two. + +- [ ] **Step 2: The padding test.** Create `tslang/test/tester/tests/00union_member_padding.ts`. The shapes below are chosen so that one member's field falls in another member's padding: at x64 for the first union, and at i686 for the second. + +```ts +// A union is stored as one byte buffer. Each member must read back exactly what it was +// stored with, even when its fields sit where another member's struct has padding. + +type Three = { flag: boolean; a: s32; b: s32 }; +type Big = { flag: boolean; v: number }; +type U1 = Three | Big; + +function makeThree(): U1 { return { flag: true, a: 7, b: 9 }; } +function makeBig(): U1 { return { flag: false, v: 1.5 }; } + +type Loading = { state: string }; +type Failed = { state: string; code: number }; +type Success = { state: string; response: { title: string; duration: number; summary: string } }; +type Net = Loading | Failed | Success; + +function describe(s: Net): string { + if (s.state == "failed") return `code ${(s).code}`; + if (s.state == "success") return `title ${(s).response.title}`; + return "loading"; +} + +function main() { + const t = makeThree(); + const t2 = t; // copy the union as a value + const three = t2; + assert(three.a == 7); + assert(three.b == 9); + + const b = makeBig(); + const b2 = b; + assert((b2).v == 1.5); + + const f: Net = { state: "failed", code: 1.0 }; + const f2 = f; + assert(describe(f2) == "code 1"); + assert(describe({ state: "success", response: { title: "t", duration: 2, summary: "s" } }) == "title t"); + assert(describe({ state: "loading" }) == "loading"); + + print("done."); +} +``` + + - Use `u` to get a member back. Phase 2 found that `u.field` narrowing of object unions reads through the first member. + - If a construct is rejected, adapt it, keeping two things: some union member's field lies in another member's padding at x64, and some lies in padding at i686. Put a comment next to each union saying which member's field lies in which padding, at which arch. + - Register it in `tslang/test/tester/CMakeLists.txt` next to `00bigint_struct_layout`, following that entry's pattern. + +- [ ] **Step 3: Re-arm `00bigint_struct_layout`.** It currently documents that it cannot assert `a`. Find that comment and add the assert back. + +- [ ] **Step 4: Confirm the tests fail.** + - Build them at x64 and i686 under gc, rc and none with the current compiler. At least one assert must fail at x64 (the `a` field), and `describe(f2)` must fail at i686. + - Record which assert fails where. If nothing fails at x64, the first union shape is wrong for x64: change it until it fails, and explain the layout in the comment. + - `ctest -j 16 -C Release` now has the new failures, and nothing else. + +- [ ] **Step 5: Commit.** Commit the script and tests as they are. The suite is red on the two union tests until Task 2; say so in the commit message. + +--- + +### Task 2: Byte-array union storage + +**Files:** +- Modify: `tslang/lib/TypeScript/LowerToLLVM.cpp`: + - the `UnionType` conversion, near line 6690; + - `CreateUnionInstanceOpLowering` and `GetValueFromUnionOpLowering`, near lines 2123-2230. +- Modify: `tslang/include/TypeScript/LowerToLLVM/LLVMTypeConverterHelper.h` +- Audit, and modify where needed: + - `tslang/include/TypeScript/LowerToLLVM/CastLogicHelper.h` (lines ~570, ~623, ~1121: the three `findMaxSizeType` callers, and the struct-to-struct path ~790-815); + - `OwnershipRoutineLogic.h` (GEPs to `UNION_VALUE_INDEX` at ~941 and ~1095); + - `UnaryBinLogicalOrHelper.h`; + - `LLVMDebugInfo.h`; + - every other use of `UNION_VALUE_INDEX` or `findMaxSizeType`. + +**Interfaces:** +- Consumes: Task 1's tests and `probe-corpus.sh`. +- Produces: + - `LLVMTypeConverterHelper::getUnionStorageSize(mlir_ts::UnionType) -> unsigned`: the largest member's alloc size, in bytes. + - The union's LLVM type becomes packed `{ptr, [N x i8]}` when it needs a tag. When it doesn't, it is the merged base type, unchanged: `isUnionTypeNeedsTag` false means there is no real union. + +- [ ] **Step 1: Storage size.** In `LLVMTypeConverterHelper`, add: + +```cpp + // Bytes a union's value field must hold: the largest alloc size among its members. + unsigned getUnionStorageSize(mlir_ts::UnionType unionType) + { + unsigned size = 0; + for (auto subType : unionType.getTypes()) + { + size = std::max(size, (unsigned)getTypeAllocSizeInBytes(typeConverter->convertType(subType))); + } + + return size; + } +``` + +Keep `findMaxSizeType` for now. Step 3 decides whether any caller still needs a member *type*, not only a size. + +- [ ] **Step 2: The conversion.** In the `UnionType` conversion, when a tag is needed, use `LLVM::LLVMArrayType::get(th.getI8Type(), ltch.getUnionStorageSize(type))` as the value field instead of `selectedType`. Put a short comment above it that states the reason: the padding of a member struct is not carried by a value copy, and at 32-bit a member's `f64` sits in the storage member's padding. The no-tag path stays as it is. + +- [ ] **Step 3: Audit every reader.** For each use of `UNION_VALUE_INDEX`, `findMaxSizeType` and the union's converted type, write down in your report what it does and whether a byte array breaks it. + - **Expected safe.** + - `CreateUnionInstanceOp` and `GetValueFromUnionOp` build a packed `{ptr, member}` and move it through memory with `castLLVMTypes`, and `CopyStructOp` copies `min(src, dst)` bytes. + - Ownership GEPs to the value field and then load or store the member type through that pointer. + - These are memory reinterpretations, which are correct for any storage type of the same size. Confirm each one. + - **Expected to need a change.** + - The union→union cast in `CastLogicHelper.h` (~623) passes `findMaxSizeType(inUnionType)` as a value type. It would now carry the source's *member* type into a different union. Use the source union's byte-array type instead. Check that `castLLVMTypes` handles array→array and array→struct through memory. Its struct-to-struct branch only matches `LLVMStructType` on both sides: extend it to cover `LLVMArrayType` as well, using the same memory path. + - The ~1121 caller: read it and decide. + - `LLVMDebugInfo.h`: describe the value field as a byte array, or as the members over the same offset. Keep `--di` builds compiling and running; the suite runs a `--di` variant. + - **Anything else** that extracts the value field *as a value* and uses it as a member struct without going through memory is a bug under this change. Route it through memory as `GetValueFromUnionOp` does. + +- [ ] **Step 4: Build and run the Task 1 tests.** `00union_member_padding` and `00bigint_struct_layout` must pass at x64 and i686 under gc, rc and none. Build them at i686 with `--emit=llvm` too, and confirm there are no `types have different sizes` warnings for those unions. The warning compares struct sizes, which are now equal by construction, or arrays, which are no longer compared. + +- [ ] **Step 5: Verify everything.** + - `ctest -j 16 -C Release` passes everything: 2766 plus the new test. + - `probe-corpus.sh` after the change, then `--compare `: nothing that passed before fails after. Report the files that pass *now* and did not before; `02union_type`, `03union_type` and `03union_type_case_order` at i686 are expected among them. List the remaining i686-only failures from the phase 4 measurement that still fail, with one line each on whether they look union-related. + - rc: run `--verify-ownership` on the rc corpus files that use tagged unions. The union files in the corpus are the ones whose `--emit=mlir` output contains `ts.CreateUnionInstance`. The verifier must stay silent where it was silent at the base commit. + - **x64 code size:** total `.text` size of the corpus exes before and after (`llvm-readobj --sections`), or the byte count of `--emit=obj` objects if that's simpler. Report it; it's not gated. + +- [ ] **Step 6: Commit.** Use one commit for the storage change and its audit fixes, with a message explaining the defect and the fix. + +--- + +## Phase 4a gate + +- [ ] `00union_member_padding` and `00bigint_struct_layout` (with `a` asserted) pass at x64 and i686 under gc, rc and none. +- [ ] `ctest -j 16 -C Release` passes everything. +- [ ] `probe-corpus.sh --compare` against the base-commit baseline: no new failures at either arch. +- [ ] `02union_type` passes at i686. The fate of every other i686-only failure is reported. +- [ ] x64 code-size change reported. + +## Rulings made while writing this plan + +- **`[N x i8]`, not an array of words.** Alignment is irrelevant because the union struct is already packed. A byte array is the one storage type with no padding at every size. If the measured code size shows a real regression, an `[N/8 x i64]`-plus-tail layout can follow in a separate change. +- **`findMaxSizeType` stays** until the audit shows no caller needs a member type. Deleting it is not the goal. +- **The phase 2 union ruling is reversed.** Phase 2 deferred this defect as x64-only and rare. At i686 it breaks corpus tests, so it blocks phase 4's gate. diff --git a/docs/superpowers/specs/2026-09-20-32-bit-compilation-design.md b/docs/superpowers/specs/2026-09-20-32-bit-compilation-design.md index 914034d73..0e07ab109 100644 --- a/docs/superpowers/specs/2026-09-20-32-bit-compilation-design.md +++ b/docs/superpowers/specs/2026-09-20-32-bit-compilation-design.md @@ -3,7 +3,7 @@ Design for compiling TypeScript to 32-bit targets, with `i686-pc-windows-msvc` as the proof target and width-correctness that generalizes to any 32-bit triple. -Status: approved design. Phases 1-3 implemented; phase 4 not. +Status: approved design. Phases 1-3 and 4a implemented; 4b and 4c not. ## Why now @@ -261,6 +261,29 @@ Already found: at i686, `00for_await` and `00for_await_yield` fail at failures are triaged and recorded the way the Debug-suite failures are, not silently excluded. +**Split.** A corpus probe (`tslang/test/probe-corpus.sh`: every corpus file as +a single-file exe, x64 and i686 × gc/rc/none) found 15 files failing at i686 +only, so phase 4 is three PRs: + +- **4a — union storage as bytes (done).** A tagged union stored its value as + its largest member's struct; bytes in that struct's padding were not carried + by a value copy, so another member's field there was lost — at x64 for some + layouts, at i686 for common ones (a pointer followed by an 8-aligned `f64`). + The value field is now a packed `<{[N/P x iP], [N%P x i8]}>` (P the pointer + size), with the unused tail zeroed when a smaller member is stored. Also + fixed: `castLLVMTypes` copied small aggregates as one pointer (4 bytes at + i686), and a module-level union initialized with a constant now gets a + global constructor instead of failing to compile. 29 probe rows newly pass, + none regress; x64 `.text` is unchanged within 0.03%. +- **4b — async at i686.** +- **4c — the suite at i686:** test-runner arch flag, x86 default library, + reading `__decls` from a DLL file, and the remaining failures. + +Remaining i686-only probe failures after 4a: the six async files (4b); +`internals` (an `inline_asm` with an `=r` constraint, which no single +i686 register can satisfy); `02funcs_vararg` (unresolved `_printf`; it is +commented out of the suite). + ## Error handling Three failures get explicit, actionable messages instead of downstream @@ -312,6 +335,12 @@ confusion: process exits 127 with its earlier output unflushed. `throw x` and class objects are caught. Pre-existing; `eh_order.ts` uses `catch (e: TypeOf<1>)`. +- **A global initialized from a DLL-exported global reads garbage (not 32-bit; + x64 too).** Found in phase 4a review: in an importer, `const copiedN: number + = cN` where `cN` is exported by a `-shared` module prints a denormal (e.g. + 3.04e-312) with both the old and new compiler; a union copy crashes silently. + Likely the importer's global constructor runs before the import is bound. + ## Out of scope - `-m32` shorthand. `-mtriple=i686-pc-windows-msvc` already works and is the diff --git a/tslang/include/TypeScript/LowerToLLVM/CastLogicHelper.h b/tslang/include/TypeScript/LowerToLLVM/CastLogicHelper.h index a89628af4..500eef348 100644 --- a/tslang/include/TypeScript/LowerToLLVM/CastLogicHelper.h +++ b/tslang/include/TypeScript/LowerToLLVM/CastLogicHelper.h @@ -560,23 +560,6 @@ class CastLogicHelper } } - /* - // TODO: we do not need as struct can cast to struct - if (auto inUnionType = dyn_cast(inType)) - { - if (auto resUnionType = dyn_cast(resType)) - { - LLVMTypeConverterHelper ltch((LLVMTypeConverter &)tch.typeConverter); - auto maxStoreType = ltch.findMaxSizeType(inUnionType); - auto value = rewriter.create(loc, maxStoreType, in); - auto typeOfValue = - rewriter.create(loc, mlir_ts::StringType::get(rewriter.getContext()), in); - auto unionValue = rewriter.create(loc, resType, value, typeOfValue); - return unionValue; - } - } - */ - if (auto resUnionType = dyn_cast(resType)) { // TODO: do I need to test income types? @@ -619,9 +602,11 @@ class CastLogicHelper bool needTagRes = mth.isUnionTypeNeedsTag(loc, resUnionType, baseTypeRes); if (needTagRes) { + // carry the source's value field as it is stored - its bytes, whichever + // member they hold - not as one member's type, whose padding a value copy drops LLVMTypeConverterHelper ltch((const LLVMTypeConverter *)tch.typeConverter); - auto maxStoreType = ltch.findMaxSizeType(inUnionType); - auto value = rewriter.create(loc, maxStoreType, in); + auto storageType = ltch.getUnionStorageType(inUnionType); + auto value = rewriter.create(loc, storageType, in); auto typeOfValue = rewriter.create(loc, mlir_ts::StringType::get(rewriter.getContext()), in); auto unionValue = rewriter.create(loc, resType, value, typeOfValue); return unionValue; @@ -772,7 +757,11 @@ class CastLogicHelper return mlir::Value(); } - mlir::Value castLLVMTypes(mlir::Value in, mlir::Type inLLVMType, mlir::Type resType, mlir::Type resLLVMType) + // `sizesMayDiffer`: the caller reinterprets on purpose between aggregates of different sizes + // - a union member going into or out of a tagged union's storage - so a size mismatch is not + // worth a warning. + mlir::Value castLLVMTypes(mlir::Value in, mlir::Type inLLVMType, mlir::Type resType, mlir::Type resLLVMType, + bool sizesMayDiffer = false) { if (inLLVMType == resLLVMType) { @@ -795,7 +784,7 @@ class CastLogicHelper auto srcSize = llvmtch.getTypeAllocSizeInBytes(inLLVMType); auto dstSize = llvmtch.getTypeAllocSizeInBytes(resLLVMType); - if (srcSize != dstSize) + if (srcSize != dstSize && !sizesMayDiffer) { op->emitWarning("types have different sizes:\n ") << inLLVMType << " size of #" << srcSize << ",\n " << resLLVMType << " size of #" << dstSize; @@ -805,15 +794,21 @@ class CastLogicHelper loc, mlir_ts::RefType::get(inType), in, rewriter.getBoolAttr(false), rewriter.getIndexAttr(0)); auto dstAddr = rewriter.create(loc, mlir_ts::RefType::get(resType), mlir::Value(), rewriter.getBoolAttr(false), rewriter.getIndexAttr(0)); - if (srcSize <= 8 && dstSize <= 8) - { - rewriter.create(loc, dstAddr, srcAddr); - } - else + + if (dstSize > srcSize) { - rewriter.create(loc, dstAddr, srcAddr); + // the copy fills only the first srcSize bytes: zero the rest rather than leave + // it undefined - a union's storage is compared bytewise once boxed into `any`, + // and stale bytes could look like pointers to a conservative collector + auto dstPtr = rewriter.create(loc, tch.convertType(dstAddr.getType()), dstAddr); + auto zero = rewriter.create(loc, resLLVMType); + rewriter.create(loc, zero, dstPtr); } + // copies min(srcSize, dstSize) bytes; LLVM turns a constant-size copy into plain + // loads and stores + rewriter.create(loc, dstAddr, srcAddr); + auto val = rewriter.create(loc, resType, dstAddr); return val; } @@ -1117,10 +1112,12 @@ class CastLogicHelper typeOfValue = toh.typeOfLogic(loc, valueForBoxing, unionType, compileOptions); LLVMTypeConverterHelper llvmtch((const LLVMTypeConverter *)tch.typeConverter); - // so we need to get biggest value from Union - auto maxUnionType = llvmtch.findMaxSizeType(unionType); - LLVM_DEBUG(llvm::dbgs() << "\n!! max size union type: " << maxUnionType << "\n";); - valueForBoxing = rewriter.create(loc, maxUnionType, in); + // box the union's value field whole, as its bytes: unboxing reads it back as + // whichever member the tag names, and a member type's padding would drop bytes + // another member keeps there + auto storageType = llvmtch.getUnionStorageType(unionType); + LLVM_DEBUG(llvm::dbgs() << "\n!! union storage type: " << storageType << "\n";); + valueForBoxing = rewriter.create(loc, storageType, in); } else { diff --git a/tslang/include/TypeScript/LowerToLLVM/LLVMTypeConverterHelper.h b/tslang/include/TypeScript/LowerToLLVM/LLVMTypeConverterHelper.h index ea1096b02..6100e3997 100644 --- a/tslang/include/TypeScript/LowerToLLVM/LLVMTypeConverterHelper.h +++ b/tslang/include/TypeScript/LowerToLLVM/LLVMTypeConverterHelper.h @@ -140,6 +140,44 @@ class LLVMTypeConverterHelper return selectedType; } + // Bytes a union's value field must hold: the largest alloc size among its members. + unsigned getUnionStorageSize(mlir_ts::UnionType unionType) + { + unsigned size = 0; + for (auto subType : unionType.getTypes()) + { + size = std::max(size, (unsigned)getTypeAllocSizeInBytes(typeConverter->convertType(subType))); + } + + return size; + } + + // The LLVM type of a tagged union's value field: N bytes, N being the largest member's alloc + // size, as a packed <{ [N / P x iP], [N % P x i8] }> with P the pointer size in bytes (an + // empty part is left out). Being packed, it has no padding, so every byte of every member is + // carried by a value copy (see the UnionType conversion in LowerToLLVM.cpp). Pointer-sized + // words rather than one [N x i8]: LLVM passes an array argument element by element, and a + // byte array would become N separate arguments. + mlir::Type getUnionStorageType(mlir_ts::UnionType unionType) + { + auto *context = &typeConverter->getContext(); + auto size = getUnionStorageSize(unionType); + auto wordBytes = getPointerBitwidth(0) / 8; + + SmallVector parts; + if (size / wordBytes > 0) + { + parts.push_back(LLVM::LLVMArrayType::get(mlir::IntegerType::get(context, wordBytes * 8), size / wordBytes)); + } + + if (size % wordBytes > 0) + { + parts.push_back(LLVM::LLVMArrayType::get(mlir::IntegerType::get(context, 8), size % wordBytes)); + } + + return LLVM::LLVMStructType::getLiteral(context, parts, /*isPacked=*/true); + } + const LLVMTypeConverter *typeConverter; }; } // namespace typescript diff --git a/tslang/lib/TypeScript/LowerToLLVM.cpp b/tslang/lib/TypeScript/LowerToLLVM.cpp index e1d04f663..cd54fbae3 100644 --- a/tslang/lib/TypeScript/LowerToLLVM.cpp +++ b/tslang/lib/TypeScript/LowerToLLVM.cpp @@ -2166,7 +2166,7 @@ struct CreateUnionInstanceOpLowering : public TsLlvmPattern(loc, udefVal, transformed.getTypeInfo(), MLIRHelper::getStructIndex(rewriter, UNION_TAG_INDEX)); auto val1 = rewriter.create(loc, val0, in, MLIRHelper::getStructIndex(rewriter, UNION_VALUE_INDEX)); - auto casted = castLogic.castLLVMTypes(val1, unionPartialType, op.getType(), resType); + auto casted = castLogic.castLLVMTypes(val1, unionPartialType, op.getType(), resType, /*sizesMayDiffer=*/true); if (!casted) { return mlir::failure(); @@ -2211,7 +2211,7 @@ struct GetValueFromUnionOpLowering : public TsLlvmPatterncompileOptions); auto casted = castLogic.castLLVMTypes(transformed.getIn(), transformed.getIn().getType(), unionPartialType, - unionPartialType); + unionPartialType, /*sizesMayDiffer=*/true); if (!casted) { return mlir::failure(); @@ -4025,7 +4025,10 @@ struct GlobalOpLowering : public TsLlvmPattern isa(op) || isa(op) || isa(op) || isa(op) || isa(op) || isa(op) || isa(op) || isa(op) || - isa(op) || isa(op)) + isa(op) || isa(op) || + // a tagged union's value goes in and out through memory (an alloca and a copy), + // which a global's initializer cannot hold + isa(op) || isa(op)) { createAsGlobalConstructor = true; } @@ -4036,6 +4039,22 @@ struct GlobalOpLowering : public TsLlvmPattern { createAsGlobalConstructor = true; } + else + { + // see CreateUnionInstanceOp above: a cast to or from a tagged union lowers to + // one of those ops + MLIRTypeHelper mth(rewriter.getContext(), tsLlvmContext->compileOptions); + for (auto type : {castType, castOp.getIn().getType()}) + { + if (auto unionType = dyn_cast(type)) + { + if (mth.isUnionTypeNeedsTag(castOp->getLoc(), unionType)) + { + createAsGlobalConstructor = true; + } + } + } + } } // TODO: error in try-catch @@ -6691,10 +6710,18 @@ static void populateTypeScriptConversionPatterns(LLVMTypeConverter &converter, m LLVMTypeConverterHelper ltch(&converter); MLIRTypeHelper mth(m.getContext(), compileOptions); - mlir::Type selectedType = ltch.findMaxSizeType(type); bool needTag = mth.isUnionTypeNeedsTag(mlir::UnknownLoc::get(type.getContext()), type); - LLVM_DEBUG(llvm::dbgs() << "\n!! max size type in union: " << selectedType + // With a tag, the value field is padding-free storage as large as the largest member + // (packed words plus a byte tail, see getUnionStorageType), not that member's struct + // type. A value copy of a struct does not carry its padding, and another member's field + // can sit in that padding: at 32-bit a member's f64 lands in the storage member's padding + // after a 4-byte pointer, and it read back as garbage once the union was copied. Without + // a tag there is no real union - the members share one base type - and that type is kept + // as it is. + mlir::Type selectedType = needTag ? ltch.getUnionStorageType(type) : ltch.findMaxSizeType(type); + + LLVM_DEBUG(llvm::dbgs() << "\n!! storage type in union: " << selectedType << "\n size: " << ltch.getTypeAllocSizeInBytes(selectedType) << "\n Tag: " << (needTag ? "yes" : "no") << "\n union type: " << type << "\n";); @@ -7222,8 +7249,8 @@ void TypeScriptToLLVMLoweringPass::runOnOperation() // module. LLVM's default layout - what this used before - has 8-byte pointers and a 4-byte-aligned // i64, wrong for i686 in the first and for x64 in the second, so a module without the attribute is // an error rather than a fallback. SizeOfOp does not read this layout (it emits getelementptr - // null, 1, which LLVM folds with the module's layout), but union storage selection - // (findMaxSizeType), getIntPtrType and debug-info offsets do. Checked before anything else, so + // null, 1, which LLVM folds with the module's layout), but union storage sizing + // (getUnionStorageSize), getIntPtrType and debug-info offsets do. Checked before anything else, so // the pass fails before it has changed the module (the @dllname renames below). auto dataLayoutAttr = m->getAttrOfType(mlir::LLVM::LLVMDialect::getDataLayoutAttrName()); if (!dataLayoutAttr) diff --git a/tslang/test/probe-corpus.sh b/tslang/test/probe-corpus.sh new file mode 100755 index 000000000..45338f52d --- /dev/null +++ b/tslang/test/probe-corpus.sh @@ -0,0 +1,107 @@ +#!/usr/bin/env bash +# Corpus probe: compile and run every file in tslang/test/tester/tests as a single-file exe, for +# x64 and i686 under the gc, rc and none memory models, with the suite's flags. Hand-run, like the +# check-*.sh scripts; the result is a TSV to compare against a baseline taken before a change. +# +# probe-corpus.sh [jobs] +# Writes /results.tsv with the columns: name arch mm stage code +# stage is pass, compile, link, run or timeout; code is the compile or run exit code. +# +# probe-corpus.sh --compare +# Prints every row that passed before and does not pass after, then a summary line. +# Exits non-zero if there is any such row. +# +# About 100 files per model fail at x64 too: they are multi-file tests that need the test runner. +# "No new failures" is judged against a baseline, not against zero. +set -u + +if [ "${1:-}" = "--compare" ]; then + before="${2:?usage: probe-corpus.sh --compare }" + after="${3:?usage: probe-corpus.sh --compare }" + for f in "$before" "$after"; do + [ -f "$f" ] || { echo "FAIL missing prerequisite: $f"; exit 2; } + done + awk -F'\t' ' + NR == FNR { if ($4 == "pass") passed[$1 FS $2 FS $3] = 1; next } + { key = $1 FS $2 FS $3; seen[key] = 1 + if ((key in passed) && $4 != "pass") { print "REGRESSED\t" $0; bad++ } + if ($4 == "pass" && !(key in passed)) fixed++ } + END { + for (k in passed) if (!(k in seen)) { split(k, p, FS); print "MISSING\t" p[1] "\t" p[2] "\t" p[3]; bad++ } + printf "summary: %d passed before and not after, %d newly passing\n", bad + 0, fixed + 0 + exit bad > 0 ? 1 : 0 + }' "$before" "$after" + exit $? +fi + +# One case, run by xargs below: probe-corpus.sh --one +if [ "${1:-}" = "--one" ]; then + out="$2"; T="$3"; f="$4"; arch="$5"; mm="$6" + REPO="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" + name="$(basename "$f" .ts)" + case "$arch" in + x64) triple=x86_64-pc-windows-msvc + libs=(--tslang-lib-path="$REPO/__build/tslang/windows-msbuild-2026-release/lib") ;; + x86) triple=i686-pc-windows-msvc + libs=(--tslang-lib-path="$REPO/__build/tslang-runtime/release") ;; + esac + libs+=(--gc-lib-path="$REPO/3rdParty/gc/x64/release/lib") + d="$out/cases/$arch/$mm/$name" + mkdir -p "$d" + ( cd "$d" && "$T" --emit=exe --opt --opt_level=3 --no-default-lib --entry-point -mm="$mm" \ + -mtriple="$triple" "${libs[@]}" "$f" -o "$d/$name.exe" > "$d/compile.txt" 2>&1 ) + c=$? + if [ $c -ne 0 ]; then + if grep -q "LNK" "$d/compile.txt"; then stage=link; else stage=compile; fi + printf '%s\t%s\t%s\t%s\t%s\n' "$name" "$arch" "$mm" "$stage" "$c" + else + ( cd "$d" && timeout 30 "$d/$name.exe" > "$d/run.txt" 2>&1 ) + r=$? + if [ $r -eq 0 ]; then stage=pass; elif [ $r -eq 124 ]; then stage=timeout; else stage=run; fi + printf '%s\t%s\t%s\t%s\t%s\n' "$name" "$arch" "$mm" "$stage" "$r" + fi + rm -f "$d"/*.exe "$d"/*.obj "$d"/*.o "$d"/*.pdb "$d"/*.ilk "$d"/*.lib "$d"/*.exp 2>/dev/null + exit 0 +fi + +TSLANG="${1:?usage: probe-corpus.sh [jobs] | --compare }" +OUT="${2:?usage: probe-corpus.sh [jobs]}" +JOBS="${3:-12}" +SELF="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/$(basename "${BASH_SOURCE[0]}")" +REPO="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +TESTS="$REPO/tslang/test/tester/tests" + +# These point at a stale install on some machines and would override the explicit lib flags. +unset GC_LIB_PATH TSLANG_LIB_PATH GC_SHARED_LIB_PATH DEFAULT_LIB_PATH + +missing=0 +need() { [ -e "$1" ] || { echo "FAIL missing prerequisite: $1"; missing=1; }; } +need "$TSLANG" +need "$TESTS" +need "$REPO/3rdParty/gc/x64/release/lib/gc.lib" +need "$REPO/3rdParty/gc/x64/release/lib/x86" +need "$REPO/__build/tslang/windows-msbuild-2026-release/lib/TypeScriptAsyncRuntime.lib" +need "$REPO/__build/tslang-runtime/release/x86" +command -v timeout >/dev/null 2>&1 || { echo "FAIL missing prerequisite: timeout"; missing=1; } +command -v xargs >/dev/null 2>&1 || { echo "FAIL missing prerequisite: xargs"; missing=1; } +[ $missing -eq 0 ] || exit 2 + +TSLANG="$(cd "$(dirname "$TSLANG")" && pwd)/$(basename "$TSLANG")" +mkdir -p "$OUT" +OUT="$(cd "$OUT" && pwd)" + +for f in "$TESTS"/*.ts; do + for arch in x64 x86; do + for mm in gc rc none; do + printf '%s\0%s\0%s\0' "$f" "$arch" "$mm" + done + done +done | xargs -0 -n 3 -P "$JOBS" bash "$SELF" --one "$OUT" "$TSLANG" > "$OUT/results.unsorted" + +sort "$OUT/results.unsorted" > "$OUT/results.tsv" +rm -f "$OUT/results.unsorted" + +echo "arch mm pass total" +awk -F'\t' '{ t[$2 " " $3]++; if ($4 == "pass") p[$2 " " $3]++ } + END { for (k in t) printf "%s %d %d\n", k, p[k] + 0, t[k] }' "$OUT/results.tsv" | sort +echo "wrote $OUT/results.tsv" diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 6b483761e..1e6f96ad4 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -178,6 +178,8 @@ add_test(NAME test-compile-00-const-array-to-array-elements COMMAND test-runner add_test(NAME test-compile-00-const-array-nested-elements COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_array_nested_elements.ts") add_test(NAME test-compile-00-array-move-wide-elements COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_move_wide_elements.ts") add_test(NAME test-compile-00-bigint-struct-layout COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00bigint_struct_layout.ts") +add_test(NAME test-compile-00-union-member-padding COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_member_padding.ts") +add_test(NAME test-compile-00-union-global-init COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_global_init.ts") add_test(NAME test-compile-00-funcs-expression-generic COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_expression_generic.ts") add_test(NAME test-compile-00-funcs-expression-iterator COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_expression_iterator.ts") add_test(NAME test-compile-00-arrow-generic COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00arrow_generic.ts") @@ -667,6 +669,7 @@ add_test(NAME test-jit-00-union-bin-ops COMMAND test-runner -jit "${PROJECT_SOUR add_test(NAME test-jit-00-union-bin-ops-2 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_bin_ops2.ts") add_test(NAME test-jit-00-union-to-any COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_to_any.ts") add_test(NAME test-jit-00-union-errors COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_errors.ts") +add_test(NAME test-jit-00-union-global-init COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_global_init.ts") add_test(NAME test-jit-00-intersection-type COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00intersection_type.ts") add_test(NAME test-jit-00-intersection-type-generic COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00intersection_type_generic.ts") add_test(NAME test-jit-00-length COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00length.ts") diff --git a/tslang/test/tester/tests/00bigint_struct_layout.ts b/tslang/test/tester/tests/00bigint_struct_layout.ts index d06615d21..b394fae9f 100644 --- a/tslang/test/tester/tests/00bigint_struct_layout.ts +++ b/tslang/test/tester/tests/00bigint_struct_layout.ts @@ -99,13 +99,11 @@ function main() { const big = pickUnion(false); assert(big.flag, "union big flag"); assert(big.v == 9007199254740993n, "union big value"); - // The other member reads back only in part. `a` sits at offset 4, inside the padding of the - // {i1, i64} storage, and a struct copy does not carry padding, so it comes back 0. That is a - // separate union-storage defect, independent of the data layout: the same union with `number` - // in place of `bigint` ({i1, double}, 8-aligned under either layout) loses `a` the same way. It - // is not asserted here; `b`, outside the padding, is. + // The other member must read back whole too. `a` sits at offset 4, inside the padding of the + // {i1, i64} member; union storage has to carry those bytes (see 00union_member_padding). const three = pickUnion(true); assert(!three.flag, "union three flag"); + assert(three.a == 2, "union three a"); assert(three.b == 3, "union three b"); print("done."); diff --git a/tslang/test/tester/tests/00union_global_init.ts b/tslang/test/tester/tests/00union_global_init.ts new file mode 100644 index 000000000..1ee837675 --- /dev/null +++ b/tslang/test/tester/tests/00union_global_init.ts @@ -0,0 +1,58 @@ +// Module-level and class-static unions initialized with a constant, holding the largest member +// and a smaller one. A tagged union's value goes in through memory (an alloca and a copy), which +// a global's initializer cannot hold, so these globals are set up by a global constructor. +// +// The end of the file checks that a union's unused storage bytes are zero: two equal numbers +// boxed into `any` from `number | P` compare equal. The comparison looks at the whole boxed +// storage, so bytes the number does not cover must not be left over from an earlier P. + +let gLetBig: number | string = 2.5; +let gLetSmall: number | string = "x"; +const gConstBig: number | string = 4.5; +const gConstSmall: number | string = "y"; +let gFlag: s32 | boolean = true; + +type Three = { flag: boolean; a: s32; b: s32 }; +type Big = { flag: boolean; v: number }; +const gThree: Three | Big = { flag: true, a: 3, b: 4 }; + +class C { + static sBig: number | string = 1.5; + static sSmall: number | string = "s"; +} + +function show(u: number | string): string { + if (typeof u == "number") return `n${u}`; + return `s${u}`; +} + +type P = { a: number; b: number }; +function mkP(): number | P { return { a: 7.25, b: 9.75 }; } +function mkN(v: number): number | P { return v; } +function boxP(): any { const u = mkP(); const x: any = u; return x; } +function boxN(v: number): any { const u = mkN(v); const x: any = u; return x; } + +function main() { + assert(show(gLetBig) == "n2.5", "let, largest member"); + assert(show(gLetSmall) == "sx", "let, smaller member"); + assert(show(gConstBig) == "n4.5", "const, largest member"); + assert(show(gConstSmall) == "sy", "const, smaller member"); + assert(show(C.sBig) == "n1.5", "static, largest member"); + assert(show(C.sSmall) == "ss", "static, smaller member"); + assert(typeof gFlag == "boolean", "s32 | boolean tag"); + + const three = gThree; + assert(three.flag, "global union three.flag"); + assert(three.a == 3, "global union three.a"); + assert(three.b == 4, "global union three.b"); + + gLetSmall = 8.5; + assert(show(gLetSmall) == "n8.5", "let, reassigned"); + + const p = boxP(); + const x = boxN(1.5); + const y = boxN(1.5); + assert(x === y, "equal numbers boxed from unions"); + + print("done."); +} diff --git a/tslang/test/tester/tests/00union_member_padding.ts b/tslang/test/tester/tests/00union_member_padding.ts new file mode 100644 index 000000000..994bbbe8d --- /dev/null +++ b/tslang/test/tester/tests/00union_member_padding.ts @@ -0,0 +1,53 @@ +// A union is stored as one byte buffer. Each member must read back exactly what it was +// stored with, even when its fields sit where another member's struct has padding. +// +// A tagged union lowers to { tag, S }, where S is the type of its largest member. If S is that +// member's struct type, a smaller member's field that falls in S's padding is not part of the +// loaded value, so it is lost the next time the union is copied as a value. + +// Three is { i1, i32, i32 } (12 bytes, 4-aligned); Big is { i1, double } (16 bytes, 8-aligned) at +// both x64 and i686, so Big is the storage. Three.a (offset 4) lies in Big's padding (offsets 1-7) +// at x64 and at i686. Three.b (offset 8) overlaps Big.v and survives. +type Three = { flag: boolean; a: s32; b: s32 }; +type Big = { flag: boolean; v: number }; +type U1 = Three | Big; + +function makeThree(): U1 { return { flag: true, a: 7, b: 9 }; } +function makeBig(): U1 { return { flag: false, v: 1.5 }; } + +// At x64 every member is pointer-and-double sized, with no padding, and Success is the storage. +// At i686 Success is { ptr, pad4, { ptr, pad4, double, ptr } } (32 bytes) and Failed is +// { ptr, pad4, double }: the upper half of Failed.code (offsets 12-15) lies in the padding after +// Success.response.title, so `code` reads garbage at i686. +type Loading = { state: string }; +type Failed = { state: string; code: number }; +type Success = { state: string; response: { title: string; duration: number; summary: string } }; +type Net = Loading | Failed | Success; + +function describe(s: Net): string { + if (s.state == "failed") return `code ${(s).code}`; + if (s.state == "success") return `title ${(s).response.title}`; + return "loading"; +} + +function main() { + const t = makeThree(); + const t2 = t; // copy the union as a value + const three = t2; + assert(three.flag, "three.flag"); + assert(three.a == 7, "three.a"); + assert(three.b == 9, "three.b"); + + const b = makeBig(); + const b2 = b; + assert(!(b2).flag, "big.flag"); + assert((b2).v == 1.5, "big.v"); + + const f: Net = { state: "failed", code: 1.0 }; + const f2 = f; + assert(describe(f2) == "code 1", "describe(failed)"); + assert(describe({ state: "success", response: { title: "t", duration: 2.5, summary: "s" } }) == "title t", "describe(success)"); + assert(describe({ state: "loading" }) == "loading", "describe(loading)"); + + print("done."); +}