Skip to content

Comments

feat: add f64 floating-point support and rename ops to MLIR conventions#38

Merged
tetsuo-cpp merged 4 commits intocanonfrom
feat/float-support
Feb 20, 2026
Merged

feat: add f64 floating-point support and rename ops to MLIR conventions#38
tetsuo-cpp merged 4 commits intocanonfrom
feat/float-support

Conversation

@tetsuo-cpp
Copy link
Owner

@tetsuo-cpp tetsuo-cpp commented Feb 20, 2026

Summary

  • Implement f64 support using the bit-casting approach: floats stored as i64 bit patterns on the stack, F-prefixed ops bitcast before/after arithmetic
  • Float literals, arithmetic (F+ F- F* F/), comparison (F= F< F> F<> F<= F>=), memory (F@ F! SF@ SF!), conversion (S>F F>S), and f64 kernel params
  • Rename dialect ops to follow MLIR conventions: type-suffixed mnemonics (addi/addf, loadi/loadf, eqi/eqf, etc.) and CamelCase class names (AddIOp, AddFOp, LoadIOp, LoadFOp, etc.). literalconstant.
  • Update warpforth-runner to handle f64/f64[] params via std::variant

Test plan

  • cmake --build build --target check-warpforth passes all new and existing tests
  • New translation tests: float-literals, float-arithmetic, float-comparison, float-conversion, float-memory, float-params
  • New conversion tests: float-arithmetic.mlir, float-memory.mlir
  • New pipeline test: float-pipeline.forth
  • GPU end-to-end tests for float kernels

Closes #6

@tetsuo-cpp tetsuo-cpp changed the title feat: add f64 floating-point support feat: add f64 floating-point support and rename ops to MLIR conventions Feb 20, 2026
Copy link
Owner Author

@tetsuo-cpp tetsuo-cpp left a comment

Choose a reason for hiding this comment

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

Code Review: feat: add f64 floating-point support and rename ops to MLIR conventions

+1228 / -454 across dialect definition, parser, conversion patterns, runner, and tests.


Overview

This PR implements f64 floating-point support for WarpForth using a bitcasting approach: floats are stored as i64 bit patterns on the untyped stack, and F-prefixed operations bitcast before/after arithmetic. Simultaneously, it renames all ops to follow MLIR conventions (literalconstant, addaddi/addf, loadloadi/loadf, etc.).

The scope covers:

  • Lexer/Parser: Float literal detection, Token::Kind::Float, BaseType enum for params/shared
  • Dialect (TableGen): New float arithmetic, comparison, memory, and conversion ops; renamed integer ops
  • ForthToMemRef conversion: Templatized BinaryArithOpConversion, BinaryCmpOpConversion, MemoryLoadOpConversion, MemoryStoreOpConversion with IsFloat/AddressSpace parameters
  • warpforth-runner: std::variant-based param types for i64/f64 scalars and arrays
  • Tests: Translation, conversion, pipeline, and GPU end-to-end tests

Strengths

  • Clean architecture: The bitcasting approach is the right call for an untyped stack — avoids fundamental changes to the stack type while cleanly supporting floats.
  • Excellent template refactoring: MemoryLoadOpConversion<ForthOp, IsFloat, AddressSpace> and MemoryStoreOpConversion eliminate significant duplication from the previous 4 separate load/store conversion structs.
  • Thorough test coverage: Tests at every level — translation (6 new .forth files), conversion (2 new .mlir files), pipeline (1 new .forth), and GPU end-to-end (8 new test functions). All existing tests updated for the rename.
  • MLIR naming conventions: The type-suffixed mnemonics (addi/addf, eqi/eqf, loadi/loadf) and CamelCase class names align well with upstream MLIR style.
  • Runner precision: std::setprecision(17) for f64 output is correct for full IEEE 754 double roundtrip fidelity.

Issues & Suggestions

1. ConstantOp value attribute is too permissive

ForthOps.td:128 — The attribute type changed from I64Attr to AnyAttr:

let arguments = (ins Forth_StackType:$input_stack, AnyAttr:$value);

This would accept any attribute (string, array, dictionary, etc.). Consider narrowing to:

AnyAttrOf<[I64Attr, F64Attr]>

or at minimum TypedAttr. This provides compile-time validation and better error messages if someone misuses the op.

2. Float literal parsing: potential edge cases

ForthToMLIR.cpp:133-147 — The isFloat function relies on strtod after checking for ./e/E. A few edge cases to consider:

  • "." alone — strtod would fail (end != start + size), so this is fine.
  • "1."strtod parses this as 1.0, which is correct Forth behavior.
  • "1e"strtod parses as 1 but end would point to e, so end != start + size → correctly rejected.

These all seem handled correctly. Just flagging that I checked.

3. Float comparison NaN semantics are undocumented

The float comparisons use ordered predicates (OEQ, OLT, OGT, ONE, OLE, OGE), meaning any comparison involving NaN returns false (except ONE which returns true for NaN). This is standard IEEE 754 behavior but worth a note in CLAUDE.md or the op descriptions, since Forth users might not expect this.

4. test/Conversion/ForthToMemRef/float-arithmetic.mlir — semantic oddity

%3 = forth.addf %2 : !forth.stack -> !forth.stack
%4 = forth.subf %3 : !forth.stack -> !forth.stack  // only 1 value on stack!
%5 = forth.mulf %4 : !forth.stack -> !forth.stack
%6 = forth.divf %5 : !forth.stack -> !forth.stack

After addf consumes 2 values and pushes 1, subf would underflow at runtime. This is fine as a FileCheck pattern test (only verifies IR structure), but it could confuse future readers. A comment noting this only tests IR lowering patterns would help.

5. warpforth-runner: no error handling on stoll/stod in parseParam

warpforth-runner.cpp:159-160 — The lambdas toI64 and toF64 call std::stoll / std::stod without try/catch. If the user passes malformed values (e.g. i64:abc), this throws std::invalid_argument with no helpful message. Consider wrapping with error reporting.


Minor Nits

  • The PR bundles two logically separate changes (f64 support + op rename). If this were more complex, I'd suggest splitting, but at this scale it's manageable in one PR since the rename is largely mechanical.

Verdict

This is a well-executed PR. The architecture is sound, the template refactoring reduces duplication, and test coverage is thorough. The main actionable item is tightening the ConstantOp attribute type from AnyAttr to AnyAttrOf<[I64Attr, F64Attr]>. The rest are minor polish items.

@tetsuo-cpp tetsuo-cpp merged commit a989019 into canon Feb 20, 2026
1 check passed
@tetsuo-cpp tetsuo-cpp deleted the feat/float-support branch February 20, 2026 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Floating-point support (f32/f64 types and operations)

1 participant