diff --git a/changelog.d/10566-split-unit-wide-bigint-literals.md b/changelog.d/10566-split-unit-wide-bigint-literals.md new file mode 100644 index 0000000000..66f72da008 --- /dev/null +++ b/changelog.d/10566-split-unit-wide-bigint-literals.md @@ -0,0 +1,27 @@ +**BigInt literals wider than 64 bits no longer change value in a split module.** A module compiled as more than one +codegen unit truncated every BigInt literal that needs more than 64 bits: `2n ** 70n === 1180591620717411303424n` was +`false`, the literal printed `0`, and a 97-bit negative literal became a different 64-bit number. Single-unit builds +were correct, so the gap/parity corpus (all single-unit) could not see it, while large real modules split on their own +— any split module holding a >64-bit BigInt literal (curve primes, group orders, field moduli) silently computed with +the wrong constant (#10545). + +Root cause: `crates/perry-codegen/src/dialect/types.rs`'s constant reader built every integer operand with +`IntType::const_int(v as u64, v < 0)`, which takes a single 64-bit word, so an `i128` operand kept only its low word. +Every BigInt literal that fits in `i128` lowers to exactly those operands (`NativeRep::SmallBigInt` → +`trunc i128 C to i64` / `ashr i128 C, 64`), and the in-process dialect reader is the default only for split modules — +LLVM's own assembler, used on the single-unit and external-clang paths, reads the literal at arbitrary precision and +then extends or truncates it to the operand width. Same class as #8228/#8241: a form the closed-set reader gets wrong +is invisible to every per-PR job. + +Fix: widths above 64 bits now build both two's-complement words with `const_int_arbitrary_precision`, the non-negative +case parses as `u128` so LLVM's unsigned full-width spelling is accepted, and an integer type wider than `i128` is +refused loudly instead of silently truncated. + +Validation: a split-unit integration test (`crates/perry/tests/issue_10545_split_unit_wide_bigint_literals.rs`, which +also asserts both `unitN.native.ll` files exist so it cannot pass without the reader), a reader unit test that builds +every constant operand form codegen emits — through the typed and the line paths — and compares each against LLVM's +own parse of the same text (only the `i128` rows diverged), a unit test that drives the real emitter and checks the +words handed to `js_bigint_from_i128_parts`, and a gap test. Each fails on the parent commit. Gap suite unchanged (820 +tests, same six known non-passing); `perry-codegen` tests green; compile-time A/B of the compiler built with and +without the change: −0.07 % instructions on a module with no wide constants (byte-identical objects) and +0.005 % on a +`@noble/curves` program. diff --git a/crates/perry-codegen/src/dialect/tests.rs b/crates/perry-codegen/src/dialect/tests.rs index d1f5fab837..2186d9c935 100644 --- a/crates/perry-codegen/src/dialect/tests.rs +++ b/crates/perry-codegen/src/dialect/tests.rs @@ -495,3 +495,288 @@ entry: "inline asm lost its structural gc-leaf-function attribute:\n{printed}" ); } + +// --------------------------------------------------------------------------- +// #10545: a constant operand must mean what LLVM's own parser says it means +// --------------------------------------------------------------------------- +// +// `types::constant` is where every operand token becomes an LLVM value, on the +// typed-instruction path and the line path alike. A token it builds differently +// from LLVM's assembler fails nothing: the module verifies, runs, and computes +// with a different number — in split modules only, the reader's only default. +// #10545 was exactly that: `i128` literals kept their low 64 bits, so BigInt +// literals wider than 64 bits changed value. The tests below therefore compare +// against LLVM's parse of the SAME text rather than against expectations +// written down here. + +use crate::module::LlModule; +use crate::types::{LlvmType, DOUBLE, F32, I1, I128, I16, I32, I64, I8, PTR, VOID}; +use inkwell::values::AnyValue; + +/// `(type, token)` for every constant operand form perry-codegen emits in a +/// function body. Globals, `c"…"` byte strings and aggregate initializers are +/// deliberately absent: they live in the module skeleton, which LLVM's own +/// parser reads (`native_emit`), never this reader. +fn emitted_constant_forms() -> Vec<(LlvmType, String)> { + let mut forms: Vec<(LlvmType, String)> = Vec::new(); + macro_rules! push { + ($ty:expr, $toks:expr $(,)?) => { + forms.extend($toks.iter().map(|t: &&str| ($ty, t.to_string()))) + }; + } + push!(I1, &["true", "false", "0", "1"]); + // Narrow operands spelled past their signed range (`and i8 %f, 128`). + push!(I8, &["0", "127", "128", "255", "-1", "-128"]); + push!(I16, &["1024", "32767", "65535", "-32768"]); + push!( + I32, + &["16000000", "2147483647", "-2147483648", "4294967295", "-1"], + ); + push!( + I64, + &[ + "-1", + "9218868437227405312", + "9223372036854775807", + "-9223372036854775808", + "12345678901234567890", + "18446744073709551615", + "ptrtoint (ptr @constant_forms_global to i64)", + "undef", + "poison", + ], + ); + // `NativeRep::SmallBigInt` spells its literal with `i128::to_string`. + for v in [ + 0i128, + -1, + 64, + (1 << 64) - 1, + 1 << 64, + -(1 << 64), + 1 << 70, + -98_765_432_109_876_543_210_987_654_321, + i128::MAX, + i128::MIN, + ] { + forms.push((I128, v.to_string())); + } + // LLVM also accepts the unsigned spelling of a full-width word. + forms.push((I128, u128::MAX.to_string())); + // `nanbox::double_literal` is the emitter's only decimal spelling; cover + // each class it distinguishes (signed zero, non-finite hex, shortest + // round-trip digits, the longest expansions). + for v in [ + 0.0, + -0.0, + 1.5, + -1.0, + 0.1, + 1e21, + 1e300, + f64::MAX, + f64::MIN_POSITIVE, + 5e-324, + f64::INFINITY, + f64::NEG_INFINITY, + f64::NAN, + ] { + forms.push((DOUBLE, crate::nanbox::double_literal(v))); + } + // NaN-boxed tag words and raw bit patterns, including a signalling NaN. + push!( + DOUBLE, + &[ + "0x7FFC000000000001", + "0x7FFD000000000000", + "0x7FFF000000000000", + "0xFFF8000000000000", + "0x7FF0000000000001", + "0x7FF4000000000000", + ], + ); + push!( + F32, + &["1.5", "-0.0", "0x3FF8000000000000", "0x7FF0000000000000"] + ); + push!(PTR, &["null", "@constant_forms_global", "undef"]); + push!( + "<2 x i64>", + &[ + "", + "", + "zeroinitializer", + ], + ); + push!( + "<4 x i32>", + &["", "zeroinitializer", "poison"] + ); + forms +} + +/// One sink call per form: calls are never constant-folded by the C-API +/// builder, so the printed operand is exactly the constant that was built. +fn constant_forms_module() -> LlModule { + let forms = emitted_constant_forms(); + let mut m = LlModule::new(crate::codegen::default_target_triple()); + m.add_global("constant_forms_global", I64, "0"); + for (i, (ty, _)) in forms.iter().enumerate() { + m.declare_function(&format!("constant_sink_{i}"), VOID, &[*ty]); + } + let f = m.define_function("constant_forms", VOID, vec![]); + let entry = f.create_block("entry"); + for (i, (ty, tok)) in forms.iter().enumerate() { + entry.call_void(&format!("constant_sink_{i}"), &[(*ty, tok.as_str())]); + } + entry.ret_void(); + m +} + +fn print_function(module: &inkwell::module::Module<'_>, name: &str) -> String { + module + .get_function(name) + .unwrap_or_else(|| panic!("@{name} missing")) + .print_to_string() + .to_string() +} + +fn assert_same_function_print(label: &str, llvm: &str, reader: &str) { + let diffs: Vec = llvm + .lines() + .zip(reader.lines()) + .filter(|(a, b)| a != b) + .map(|(a, b)| format!(" LLVM: {}\n reader: {}", a.trim(), b.trim())) + .collect(); + assert!( + diffs.is_empty() && llvm.lines().count() == reader.lines().count(), + "{label} built constants that differ from LLVM's own parse of the same \ + text:\n{}", + diffs.join("\n") + ); +} + +#[test] +fn constant_operands_match_llvms_own_parse_on_typed_and_line_paths() { + let m = constant_forms_module(); + let forms = emitted_constant_forms().len(); + let function = m + .deduped_function_refs() + .into_iter() + .find(|f| f.name == "constant_forms") + .expect("fixture function"); + let text = function.to_ir(); + let header = text.lines().next().expect("define header"); + + // Reference: LLVM's assembler over the complete module text. + let llvm_ctx = Context::create(); + let llvm_module = crate::inprocess::parse_ir_text(&llvm_ctx, &m.to_ir(), "forms_llvm") + .expect("LLVM parses the fixture"); + let llvm = print_function(&llvm_module, "constant_forms"); + assert_eq!( + llvm.matches("call void @constant_sink_").count(), + forms, + "fixture lost sink calls, so some forms would go uncompared:\n{llvm}" + ); + + // Typed path: `FnStream::item`, what split modules stream. + let typed_ctx = Context::create(); + let typed_module = crate::inprocess::parse_ir_text(&typed_ctx, &m.skeleton_ir(), "forms_typed") + .expect("skeleton parses"); + let mut stream = FnStream::begin(&typed_ctx, &typed_module, header).expect("begin"); + function + .for_each_final_item::(&mut |item| stream.item(&item)) + .unwrap_or_else(|e| panic!("typed construction: {e:#}")); + let (typed, _) = stream.finish().expect("finish"); + assert!(typed >= forms, "only {typed} typed instructions were built"); + typed_module + .verify() + .unwrap_or_else(|e| panic!("verifier rejected typed module:\n{}", e.to_string())); + assert_same_function_print( + "typed path", + &llvm, + &print_function(&typed_module, "constant_forms"), + ); + + // Line path: what personality/stack-map functions stream. + let line_ctx = Context::create(); + let line_module = crate::inprocess::parse_ir_text(&line_ctx, &m.skeleton_ir(), "forms_line") + .expect("skeleton parses"); + predeclare_function_from_text(&line_ctx, &line_module, &text).expect("predeclare"); + add_function_from_text(&line_ctx, &line_module, &text) + .unwrap_or_else(|e| panic!("line construction: {e:#}")); + line_module + .verify() + .unwrap_or_else(|e| panic!("verifier rejected line module:\n{}", e.to_string())); + assert_same_function_print( + "line path", + &llvm, + &print_function(&line_module, "constant_forms"), + ); +} + +/// #10545 from the real emitter: a BigInt literal that fits `i128` lowers to +/// `NativeRep::SmallBigInt` and is boxed by splitting it into two words +/// (`trunc i128 C to i64`, `ashr i128 C, 64`). The C-API builder folds both at +/// construction, so the built call's operands ARE the words the runtime gets. +#[test] +fn wide_bigint_literal_words_survive_native_construction() { + let literals: [i128; 3] = [ + 1 << 70, + -98_765_432_109_876_543_210_987_654_321, + i128::MIN + 1, + ]; + let mut m = Module::new("wide_bigint_literals.ts"); + m.init = literals + .iter() + .enumerate() + .map(|(i, v)| Stmt::Let { + id: 4100 + i as u32, + name: format!("b{i}"), + ty: Type::BigInt, + mutable: false, + init: Some(Expr::BigInt(v.to_string())), + }) + .collect(); + m.init_kind = ModuleInitKind::Eager; + let opts = CompileOptions { + emit_ir_only: true, + is_entry_module: true, + ..Default::default() + }; + let ir = String::from_utf8(compile_module(&m, opts).expect("module compiles")) + .expect("LLVM IR is UTF-8"); + for v in literals { + assert!( + ir.contains(&format!("i128 {v}")), + "fixture no longer lowers {v}n to an i128 operand, so this test \ + would construct nothing relevant:\n{ir}" + ); + } + + let (skeleton, fns) = split_corpus(&ir); + let ctx = Context::create(); + let module = crate::inprocess::parse_ir_text(&ctx, &skeleton, "wide_bigint_skel") + .expect("skeleton parses"); + for f in &fns { + predeclare_function_from_text(&ctx, &module, f).expect("predeclare"); + } + for f in &fns { + add_function_from_text(&ctx, &module, f).unwrap_or_else(|e| panic!("{e:#}")); + } + module + .verify() + .unwrap_or_else(|e| panic!("verifier rejected native module:\n{}", e.to_string())); + let printed = module.print_to_string().to_string(); + for v in literals { + let words = format!( + "@js_bigint_from_i128_parts(i64 {}, i64 {})", + v as i64, + (v >> 64) as i64 + ); + assert!( + printed.contains(&words), + "{v}n was not boxed from its two's-complement words `{words}`:\n{printed}" + ); + } +} diff --git a/crates/perry-codegen/src/dialect/types.rs b/crates/perry-codegen/src/dialect/types.rs index c2e67ecb74..8aaa919fda 100644 --- a/crates/perry-codegen/src/dialect/types.rs +++ b/crates/perry-codegen/src/dialect/types.rs @@ -9,7 +9,7 @@ use anyhow::{anyhow, bail, Result}; use inkwell::context::Context; use inkwell::module::Module; -use inkwell::types::{BasicType, BasicTypeEnum, FunctionType, VectorType}; +use inkwell::types::{BasicType, BasicTypeEnum, FunctionType, IntType, VectorType}; use inkwell::values::BasicValueEnum; use inkwell::AddressSpace; @@ -296,16 +296,48 @@ pub(super) fn constant<'ctx>( .into() } } - BasicTypeEnum::IntType(t) => { - let v: i128 = tok.parse().map_err(|_| anyhow!("bad integer `{tok}`"))?; - t.const_int(v as u64, v < 0).into() - } + BasicTypeEnum::IntType(t) => return int_constant(t, tok), BasicTypeEnum::VectorType(t) => return vector_constant(ctx, module, t, tok), other => bail!("cannot materialize `{tok}` as {other:?}"), }, }) } +/// A decimal integer literal at its operand's width, with the text parser's +/// semantics: LLVM reads the literal at arbitrary precision, then sign-extends +/// (negative) or zero-extends (non-negative) and truncates it to the width. +/// +/// `IntType::const_int` takes ONE 64-bit word, and this used to hand it +/// `v as u64` for every width — so an `i128` operand kept only its low word. +/// `NativeRep::SmallBigInt` lowers every BigInt literal that fits in `i128` +/// to exactly such an operand (`trunc i128 1180591620717411303424 to i64`, +/// `ashr i128 …, 64`), and this reader is the default only for split modules: +/// in a split module `2n ** 70n` stopped equalling the literal `2n ** 70n`, +/// which read back as `0n`, while every single-unit build was right (#10545). +/// Widths above 64 now build the full two's-complement words. +fn int_constant<'ctx>(t: IntType<'ctx>, tok: &str) -> Result> { + let bad = || anyhow!("bad integer `{tok}`"); + // (low word, high word, negative). A non-negative literal may use the + // whole unsigned range of an `i128`, as LLVM's assembler accepts. + let (low, high, negative) = if tok.starts_with('-') { + let v: i128 = tok.parse().map_err(|_| bad())?; + (v as u64, (v >> 64) as u64, true) + } else { + let v: u128 = tok.parse().map_err(|_| bad())?; + (v as u64, (v >> 64) as u64, false) + }; + let width = t.get_bit_width(); + if width <= 64 { + return Ok(t.const_int(low, negative).into()); + } + // `basic_type` names no integer wider than `i128`; a wider one would need + // sign extension past the two words parsed above, so refuse it loudly. + if width > 128 { + bail!("integer literal `{tok}` for i{width}: the reader builds at most i128"); + } + Ok(t.const_int_arbitrary_precision(&[low, high]).into()) +} + /// `` — an LLVM constant vector literal. /// /// Perry emits one as the seed operand of the #8122/#8204 header-image diff --git a/crates/perry/tests/issue_10545_split_unit_wide_bigint_literals.rs b/crates/perry/tests/issue_10545_split_unit_wide_bigint_literals.rs new file mode 100644 index 0000000000..374cfe847d --- /dev/null +++ b/crates/perry/tests/issue_10545_split_unit_wide_bigint_literals.rs @@ -0,0 +1,107 @@ +//! Regression for #10545: a BigInt literal wider than 64 bits lost its high +//! bits whenever its module was compiled as more than one codegen unit. +//! +//! Split modules are the only default user of the in-process IR reader +//! (`perry_codegen::native_emit::native_units_mode`), and every gap/parity +//! fixture is a single unit, so the gap suite cannot see this class. Large real +//! modules split on their own; `PERRY_CODEGEN_UNITS=2` forces it here. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn runtime_dir() -> PathBuf { + std::env::var_os("PERRY_RUNTIME_DIR") + .map(PathBuf::from) + .unwrap_or_else(|| { + perry_bin() + .parent() + .expect("compiler directory") + .to_path_buf() + }) +} + +const SOURCE: &str = r#" +function f() { const x = 2n ** 70n; return x === 1180591620717411303424n; } +function g() { return 1180591620717411303424n; } +console.log(f(), g() === 2n ** 70n, String(g())); +const small = 12345678901234567890n; +const neg = -98765432109876543210987654321n; +console.log(String(small), String(neg), 2n ** 64n === 18446744073709551616n); +const edges = [ + 170141183460469231731687303715884105727n, + -170141183460469231731687303715884105727n, + 0x1ffffffffffffffffn, + -0x10000000000000000n, +]; +console.log(edges.map(String).join(" "), edges[0] === 2n ** 127n - 1n); +const p = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2fn; +console.log(String(p % 1000000007n), p === 2n ** 256n - 2n ** 32n - 977n); +"#; + +/// `node --experimental-strip-types` on the pinned oracle (26.5.1). +const EXPECTED: &str = "true true 1180591620717411303424 +12345678901234567890 -98765432109876543210987654321 true +170141183460469231731687303715884105727 -170141183460469231731687303715884105727 36893488147419103231 -18446744073709551616 true +497877021 true +"; + +#[test] +fn wide_bigint_literals_survive_a_split_module() { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("main.ts"); + let output = dir.path().join("main_bin"); + let ll_dir = dir.path().join("ll"); + std::fs::create_dir(&ll_dir).expect("create IR dump dir"); + std::fs::write(&entry, SOURCE).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .env("PERRY_NO_AUTO_OPTIMIZE", "1") + .env("PERRY_CODEGEN_UNITS", "2") + .env_remove("PERRY_LLVM_INPROCESS") + .env("PERRY_SAVE_LL", &ll_dir) + .env("PERRY_RUNTIME_DIR", runtime_dir()) + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + // The subject must be live: a build that stopped splitting, or stopped + // routing split units through native construction, would pass without + // testing the reader at all. + for unit in 0..2 { + let path = ll_dir.join(format!("main_ts.unit{unit}.native.ll")); + assert!( + path.exists(), + "expected natively constructed unit {} at {}; the module was not split \ + through the in-process reader", + unit, + path.display() + ); + } + + let run = Command::new(&output) + .current_dir(dir.path()) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + assert_eq!(String::from_utf8_lossy(&run.stdout), EXPECTED); +} diff --git a/test-files/test_gap_10545_split_unit_wide_bigint_literals.ts b/test-files/test_gap_10545_split_unit_wide_bigint_literals.ts new file mode 100644 index 0000000000..7a80f27798 --- /dev/null +++ b/test-files/test_gap_10545_split_unit_wide_bigint_literals.ts @@ -0,0 +1,58 @@ +// #10545: BigInt literals wider than 64 bits lost their high bits in a module +// compiled as more than one codegen unit (the in-process IR reader built every +// `i128` operand from its low 64-bit word). The gap harness compiles single +// units, so this file pins the semantics; the split-module witness is +// crates/perry/tests/issue_10545_split_unit_wide_bigint_literals.rs. + +// The issue repro. +function f() { const x = 2n ** 70n; return x === 1180591620717411303424n; } +function g() { return 1180591620717411303424n; } +console.log(f(), g() === 2n ** 70n, String(g())); +const small = 12345678901234567890n; +const neg = -98765432109876543210987654321n; +console.log(String(small), String(neg), 2n ** 64n === 18446744073709551616n); + +// Around the 64-bit word boundary (these always survived; kept as controls). +const words = [ + 9223372036854775807n, + -9223372036854775808n, + 18446744073709551615n, + 18446744073709551616n, + -18446744073709551616n, + -18446744073709551617n, +]; +console.log(words.map(String).join(" ")); +console.log(words[3] === words[2] + 1n, words[5] === -(2n ** 64n) - 1n); + +// Up to the i128 edge, in every radix a literal can be written in. +const edges = [ + 170141183460469231731687303715884105727n, + -170141183460469231731687303715884105727n, + 0x1ffffffffffffffffn, + 0o3777777777777777777777n, + 0b11111111111111111111111111111111111111111111111111111111111111111n, + -0x10000000000000000n, +]; +console.log(edges.map(String).join(" ")); +console.log(edges[0] === 2n ** 127n - 1n, edges[1] === -(2n ** 127n) + 1n); +console.log(edges[2] === 2n ** 65n - 1n, edges[3] === 2n ** 65n - 1n, edges[4] === 2n ** 65n - 1n); + +// Past i128: these take the string-materialized path. +const P = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2fn; +const N = 0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141n; +console.log(P === 2n ** 256n - 2n ** 32n - 977n, String(P % 1000000007n), String(N % 998244353n)); + +// Wide literals as operands, in closures, containers, class fields and defaults. +const mul = (a: bigint) => a * 1180591620717411303424n; +console.log(String(mul(3n)), String(mul(-5n) / 36893488147419103232n)); +const obj = { lo: 36893488147419103232n, hi: -73786976294838206464n }; +console.log(String(obj.lo + obj.hi), obj.lo > 18446744073709551615n, obj.hi < -18446744073709551616n); +class Field { + static readonly Q = 340282366920938463463374607431768211455n; + mask(v: bigint, m = 1267650600228229401496703205375n) { return v & m; } +} +console.log(String(Field.Q), Field.Q === 2n ** 128n - 1n, String(new Field().mask(-1n))); +console.log(1n << 100n === 1267650600228229401496703205376n, BigInt.asUintN(96, -1n) === 79228162514264337593543950335n); +let acc = 0n; +for (let i = 0; i < 5; i++) acc += 1180591620717411303424n - 1n; +console.log(String(acc), typeof 1180591620717411303424n);