Skip to content
This repository was archived by the owner on Apr 8, 2026. It is now read-only.

Add float32 IR support and scalar float lowering for the C backend#68

Merged
Vbitz merged 5 commits into
mainfrom
codex/float-ir-c-backends
Mar 7, 2026
Merged

Add float32 IR support and scalar float lowering for the C backend#68
Vbitz merged 5 commits into
mainfrom
codex/float-ir-c-backends

Conversation

@Vbitz

@Vbitz Vbitz commented Mar 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add float32 IR/frontend support alongside the existing float64 work
  • teach the IR text/binary codecs and IR printer about TY_FLOAT32, OP_CONST_F32, and float local metadata
  • add scalar float32/float64 lowering for the C backend using raw-bit transport in rtg_word, including float arithmetic, compares, jumps, conversions, and sized load/store fixes
  • document the current C-profile limitation: float64 requires c/64, and float32 requires at least c/32

Validation

  • ./build/build build
  • ./build/rtg -T ir -o - tmp/float_smoke.go
  • ./build/rtg -T c/64 tmp/float_smoke.go -o tmp/float_smoke.c
  • cc -O0 tmp/float_smoke.c -o tmp/float_smoke.bin
  • ./tmp/float_smoke.bin
  • ./build/build test-fullcompiler-c
  • ./build/build selfhost-c

@greptile-apps

greptile-apps Bot commented Mar 7, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds end-to-end float32 support (alongside the existing float64 work) across the IR type system, text/binary codecs, IR printer, Go frontend, C backend, and AArch64 backend. It is a large but well-structured change.

Key changes:

  • TY_FLOAT32 / OP_CONST_F32 added to irtypes.go; typeKindNames and opcodeNames arrays in the text codec are correctly kept in iota-index sync (including the pre-existing TY_BOOL slot).
  • Text format bumped to rtgir 3 and binary format to RTGIRB2; both codecs reject old files with clear diagnostic messages, avoiding silent mis-parse of legacy IRLocal records that lack the new float_kind field.
  • C backend uses union-based bit-transport helpers (rtg_f32_bits / rtg_bits_f32, rtg_f64_bits / rtg_bits_f64) and adds a pre-flight check rejecting float64 on c/32 and either float on c/16; the long-standing OOB read in rtg_load/rtg_store (loading RTG_WORD_BYTES bytes regardless of the requested size) is correctly fixed — v is zero-initialised so the partial-byte path is clean.
  • AArch64 backend wires up FMOV, FADD/FSUB/FMUL/FDIV/FNEG, FCMP, and FCVT* instruction helpers along with emitLdr32/emitStr32 for 4-byte memory accesses.
  • The AArch64 float comparison condition codes (COND_MI for LT, COND_LS for LEQ) are correctly chosen for IEEE 754 NaN ordering (the intuitive alternatives COND_LT/COND_LE would incorrectly return true for NaN operands), but the rationale lacks documentation to prevent future maintainer mistakes.

Confidence Score: 4/5

  • This PR is safe to merge. The implementation is correct with comprehensive float32/float64 support across all backends.
  • The core implementation is sound: codec version bumps prevent silent mis-parsing, the C backend float helpers are correct, the AArch64 instruction encodings match the ARMv8 spec. The only remaining item is a minor documentation improvement—adding clarifying comments to explain why COND_MI and COND_LS are chosen for float comparisons instead of the seemingly equivalent COND_LT and COND_LE, to prevent future NaN-handling mistakes.
  • std/compiler/backend/aarch64/backend_aarch64.go (add explanatory comments for float comparison condition codes)

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    SRC["Go source with float32/float64"]
    PARSER["parser.go\naccepts float32 type\n(removes old rejection)"]
    FRONTEND["compiler.go\nfloat32 type registered\nmergeFloatTypeNames\nemitConvertForExpr"]
    IR["IR: OP_CONST_F32 / OP_CONST_F64\nOP_ADD/SUB/MUL/DIV name=float32|float64\nOP_CONVERT Val=SRC_FLOAT32|SRC_FLOAT64\nIRLocal.FloatKind=TY_FLOAT32|TY_FLOAT64"]
    TEXT["ir_text_codec\nrtgir 3\nfloat_kind= attribute"]
    BIN["ir_binary_codec\nRTGIRB2\nFloatKind integer field"]
    CHECK["backend_c.go\ncModuleNeedsFloatKinds()\nerror if word < float width"]
    CBACK["C backend\nrtg_f32_bits / rtg_bits_f32\nrtg_f64_bits / rtg_bits_f64\nfloat arith/cmp/convert"]
    ARM64["AArch64 backend\nFMOV / FADD / FSUB / FMUL\nFDIV / FNEG / FCMP\nFCVT / SCVTF / UCVTF"]

    SRC --> PARSER --> FRONTEND --> IR
    IR --> TEXT
    IR --> BIN
    IR --> CHECK
    CHECK -->|"bits>=32 for f32\nbits>=64 for f64"| CBACK
    IR --> ARM64
Loading

Last reviewed commit: c865547

Comment thread std/compiler/binary/ir_text_codec.go
Comment thread std/compiler/backend/c/backend_c.go
@Vbitz

Vbitz commented Mar 7, 2026

Copy link
Copy Markdown
Contributor Author

@greptile review agian please.

Comment on lines +511 to +516
case ir.OP_LT, ir.OP_JMP_LT:
return COND_MI
case ir.OP_GT, ir.OP_JMP_GT:
return COND_GT
case ir.OP_LEQ, ir.OP_JMP_LEQ:
return COND_LS

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition codes chosen here are correct for IEEE 754 NaN handling, but the rationale should be documented to prevent a future maintainer from "fixing" them to the seemingly equivalent COND_LT/COND_LE.

After an AArch64 FCMP where either operand is NaN, the flags are N=0, Z=0, C=1, V=1. Under those flags:

  • COND_LT (N≠V) → 0≠1true — incorrect; Go requires NaN < x = false
  • COND_LE (Z=1 or N≠V) → false or truetrue — incorrect; Go requires NaN <= x = false

The chosen conditions correctly avoid the N≠V test:

  • COND_MI (N=1): NaN gives N=0 → false ✓
  • COND_LS (C=0 or Z=1): NaN gives C=1, Z=0 → false ✓

Consider adding a brief comment on each case:

Suggested change
case ir.OP_LT, ir.OP_JMP_LT:
return COND_MI
case ir.OP_GT, ir.OP_JMP_GT:
return COND_GT
case ir.OP_LEQ, ir.OP_JMP_LEQ:
return COND_LS
case ir.OP_LT, ir.OP_JMP_LT:
return COND_MI // N=1: correct for ordered LT; NaN gives N=0 → false (not COND_LT which triggers on N≠V, true for NaN)
case ir.OP_GT, ir.OP_JMP_GT:
return COND_GT
case ir.OP_LEQ, ir.OP_JMP_LEQ:
return COND_LS // C=0|Z=1: correct for ordered LE; NaN gives C=1,Z=0 → false (not COND_LE which triggers on N≠V, true for NaN)
case ir.OP_GEQ, ir.OP_JMP_GEQ:
return COND_GE

@Vbitz Vbitz added this pull request to the merge queue Mar 7, 2026
Merged via the queue into main with commit 36fe22e Mar 7, 2026
13 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant