Skip to content

Conversation

@Scheremo
Copy link
Contributor

@Scheremo Scheremo commented Oct 16, 2025

This commit introduces explicit support for shortreal literals in the Moore dialect and unifies the lowering and import handling of real constants. It ensures correct mapping between SystemVerilog real/shortreal and MLIR f64/f32 types across all relevant conversions.

Changes:

Dialect (MooreOps.td):

  • Added ShortrealLiteralOp:
    • Accepts F32Attr and produces RealF32.
    • Represents IEEE-754 single-precision (SystemVerilog shortreal).
  • Updated RealLiteralOp:
    • Now explicitly uses F64Attr and produces RealF64.
    • Simplified assemblyFormat by removing redundant result type.
    • Clarified description to indicate IEEE-754 double precision.

ImportVerilog (Expressions.cpp):

  • Updated materializeSVReal:
    • Emits ShortrealLiteralOp for shortreal values.
    • Emits RealLiteralOp for real values.
    • Uses appropriate FloatAttr construction and result type selection.

MooreToCore Conversion (MooreToCore.cpp):

  • Added conversion patterns:
    • RealConstantOpConv → lowers to arith.constant with f64 type.
    • ShortrealConstantOpConv → lowers to arith.constant with f32 type.
  • Marked arith::ArithDialect as legal.
  • Extended TypeConverter to map:
    • moore::RealWidth::f32mlir::Float32Type
    • moore::RealWidth::f64mlir::Float64Type

Tests:

  • Updated ImportVerilog tests to expect moore.shortreal_constant and moore.real_constant without explicit result type suffixes.
  • Added MooreToCore tests verifying correct lowering:
    • moore.real_constantarith.constant f64
    • moore.shortreal_constantarith.constant f32

@Scheremo Scheremo force-pushed the pr-lower-real-constant branch from 4410add to a6974cc Compare October 16, 2025 20:06
@Scheremo Scheremo marked this pull request as ready for review October 16, 2025 20:08
Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

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

Really nice! Just a few nits, LGTM otherwise 🥳

@Scheremo Scheremo force-pushed the pr-lower-real-constant branch 5 times, most recently from 6ba6eac to 6727d00 Compare October 17, 2025 06:03
This commit introduces explicit support for `shortreal` literals in the Moore dialect and unifies the lowering and import handling of real constants. It ensures correct mapping between SystemVerilog `real`/`shortreal` and MLIR `f64`/`f32` types across all relevant conversions.

Changes:

Dialect (MooreOps.td):
- Added `ShortrealLiteralOp`:
  - Accepts `F32Attr` and produces `RealF32`.
  - Represents IEEE-754 single-precision (SystemVerilog `shortreal`).
- Updated `RealLiteralOp`:
  - Now explicitly uses `F64Attr` and produces `RealF64`.
  - Simplified `assemblyFormat` by removing redundant result type.
  - Clarified description to indicate IEEE-754 double precision.

ImportVerilog (Expressions.cpp):
- Updated `materializeSVReal`:
  - Emits `ShortrealLiteralOp` for `shortreal` values.
  - Emits `RealLiteralOp` for `real` values.
  - Uses appropriate `FloatAttr` construction and result type selection.

MooreToCore Conversion (MooreToCore.cpp):
- Added conversion patterns:
  - `RealConstantOpConv` → lowers to `arith.constant` with `f64` type.
  - `ShortrealConstantOpConv` → lowers to `arith.constant` with `f32` type.
- Marked `arith::ArithDialect` as legal.
- Extended `TypeConverter` to map:
  - `moore::RealWidth::f32` → `mlir::Float32Type`
  - `moore::RealWidth::f64` → `mlir::Float64Type`

Tests:
- Updated `ImportVerilog` tests to expect `moore.shortreal_constant` and `moore.real_constant` without explicit result type suffixes.
- Added `MooreToCore` tests verifying correct lowering:
  - `moore.real_constant` → `arith.constant f64`
  - `moore.shortreal_constant` → `arith.constant f32`
@Scheremo Scheremo force-pushed the pr-lower-real-constant branch from 6727d00 to 79bfefe Compare October 17, 2025 06:05
Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

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

Nice 😎 LGTM!

@Scheremo Scheremo merged commit 8e02668 into llvm:main Oct 17, 2025
7 checks passed
@Scheremo Scheremo deleted the pr-lower-real-constant branch October 21, 2025 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants