fix: unblock the Xtensa f32 builtins build (LLVM constant-pool workaround) - #246
Merged
Merged
Conversation
…pool The esp Xtensa backend cannot select `XtensaISD::PCREL_WRAPPER` over a `TargetConstantPool`, so any `[N x float]` constant LLVM materialises as a constant-pool entry fails to compile for `xtensa-esp32s3-none-elf` at every optimisation level. `lps-builtins/float-f32` tripped it in `rgb2hsv`, whose `p.zw` constant lane pair LLVM emits as `[2 x float] [0.0, -1.0]`. Select the integer bit patterns and bitcast after the fact, so the constants never reach the float constant pool. `f32::to_bits` is const-evaluated, so the values are the same floats the literals were — pinned lane-by-lane against the literal form it replaced by a new test, since these builtins are shared with rv32 and wasm where the original compiles fine. The defect's guess that snoise2's gradient LUT was the trigger was wrong; it and every other suspect (`worley2/3`, `gnoise3_tile`, `psrdnoise2/3`, `snoise3`) compile unchanged. One site was enough. Defect: docs/defects/2026-08-01-xtensa-backend-cannot-select-float-constant-pool.md Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With the constant-pool workaround in place the image builds, so drop the LP_XT_BUILTINS_F32 opt-in P4 added as a stopgap and assert the f32 symbol count unconditionally, matching scripts/build-builtins.sh (rv32). Un-ignore xt_pipeline_f32's builtin-routing case. It needed more than the ignore attribute removed: the file's link_blob resolves only the module's own functions and loads at the code region's base, where the builtins image already lives, so a sym_call to __lp_lpir_ffloor_f32 could never resolve. Added a builtins-linked run path with rt_emu::xt_image's layout — image .text first, shader 4-aligned after it — hand-rolled rather than calling build_xt_image, which sits behind the `emu` feature the file's other 15 tests do not need. 16/16 green, ffloor(3.75) == 3.0 from the real builtin. Record what the defect actually was: one site (rgb2hsv), not the LUT family, and the reported constant belonged to a different function than the one that failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…raint Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…onger fits Reverts the gate flip from the previous commit, on measurement. Making --features float-f32 the default took the xtn.q32 filetest suite from 849/849 files to 522/849, and the failures are link failures, not wrong answers. Cause is capacity, not codegen. link.ld gives .text 112 KiB (114,688 B) of the emulator's 128 KiB code region. Q32-only .text is 66,300 B; with float-f32 it is 113,757 B — the image links with 931 bytes to spare, and rt_emu::xt_image places compiled shader code in exactly that gap. Small shaders fit, everything else fails to link. That is a second, independent defect from the constant-pool one, and fixing the first is what exposed it: filed as docs/defects/2026-08-01-xt-f32-builtins-exhaust-the-emulator-code-region.md with the measurements and three candidate resolutions. So the family stays behind LP_XT_BUILTINS_F32=1, and the un-ignored builtin-routing test skips loudly when the resident image lacks the f32 symbols rather than failing — one blocked capability should not redden a suite. Verified both ways: skips on the default image, and passes (ffloor(3.75) == 3.0 through the real __lp_lpir_ffloor_f32) on an image built with LP_XT_BUILTINS_F32=1. The constant-pool defect's record is corrected to say the workaround shipped and the build it blocked now succeeds, while noting the default flip is blocked elsewhere. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Yona-Appletree
changed the base branch from
claude/f32-m7-p1p2-vinst-lowering
to
main
August 1, 2026 16:02
Yona-Appletree
marked this pull request as ready for review
August 1, 2026 16:04
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #241 (
claude/f32-m7-p1p2-vinst-lowering) — it carries the M7P1/P2 wiring this needs. Merges onto/after #241; retarget to
mainonce #241lands. Note that a PR based on a
claude/*branch matches no CI trigger, sothe suite runs on retarget (this is documented in
pre-merge.yml).What was blocked
LP_XT_BUILTINS_F32=1 ./scripts/build-builtins-xt.shfailed in the backend:That build now succeeds. The gate flip does not, for a reason nobody knew
about — see below.
The workaround: one site
rgb2hsv'sp.zwconstant lane pair, which LLVM materialises as[2 x float].snoise2's gradient LUT — the defect's named suspect —compiles unchanged, as do
worley2/3,gnoise3_tile,psrdnoise2/3andsnoise3; all are present in the built image. The reported constant simplybelonged to a different function than the one that failed.
f32::from_bitsover abool-selectedu32), const-evaluated from the original literals. Chosen over anarithmetic rewrite because it is provably bit-identical — sign-of-zero is
the specific hazard arithmetic would have introduced.
verbatim in the test module as an oracle and compared lane-by-lane over a
grid crossing
g == b, plus-0.0and NaN.preference, shaped for the issue URL once filed.
The gate flip: attempted, measured, reverted
Making
--features float-f32unconditional took xtn.q32 from 849/849 filesto 522/849, on link failures. Cause is capacity, not codegen:
.textfloat-f32rt_emu::xt_imageplaces compiled shader code in exactly that gap, so smallshaders fit and everything else fails to link. That is a second, independent
defect, filed as
docs/defects/2026-08-01-xt-f32-builtins-exhaust-the-emulator-code-region.mdwith the measurements and three candidate resolutions (widen the modeled code
region / give the shader its own region / split the f32 feature so the
generative-noise family — most of the 47 KB — stays rv32-only).
So
LP_XT_BUILTINS_F32stays, with its comment rewritten to say why it nowexists. Reporting this rather than papering over it.
The pipeline test
a_builtin_routed_float_op_resolves_and_runsis no longer#[ignore]d.Removing the attribute was not enough: the file's
link_blobresolves onlythe module's own functions and loads at the code region's base, where the
builtins image already lives, so
__lp_lpir_ffloor_f32could never resolve.Added a builtins-linked run path using
rt_emu::xt_image's layout. It skipsloudly when the resident image lacks the f32 family, so one blocked capability
does not redden a suite.
Validation
LP_XT_BUILTINS_F32=1 ./scripts/build-builtins-xt.sh— 48 builtins, 82 native-f32 ✅ (was the definition of done)just check— EXIT=0cargo test -p lps-builtins --all-features— 380 passedcargo test -p lpvm-native --test xt_pipeline_f32— 16/16, both with and without the f32 imagextn.q32filetests — 6336/6336, 849/849 files (unchanged from base)float-f32is off for it)🤖 Generated with Claude Code