Skip to content

Conversation

@Scheremo
Copy link
Contributor

Extend FormatStringParser::emitInteger to correctly handle real-typed (shortreal/real) arguments when emitting %d, %h, etc. format specifiers during import.

  • Detect if the expression’s type is moore::RealType.
  • For real operands:
    • Compute required integer width based on IEEE-754 format:
      • shortreal (binary32) → 129-bit signed integer
      • real (binary64) → 1025-bit signed integer
    • Insert a moore::RealToIntOp cast before integer formatting.
  • Non-real (integral) values continue to use existing conversion logic.
  • Ensures real values print correctly via implicit real-to-integer conversion.

Extend `FormatStringParser::emitInteger` to correctly handle **real-typed (`shortreal`/`real`) arguments** when emitting `%d`, `%h`, etc. format specifiers during import.

- Detect if the expression’s type is `moore::RealType`.
- For real operands:
  - Compute required integer width based on IEEE-754 format:
    - `shortreal` (binary32) → **129-bit signed integer**
    - `real` (binary64) → **1025-bit signed integer**
  - Insert a `moore::RealToIntOp` cast before integer formatting.
- Non-real (integral) values continue to use existing conversion logic.
- Ensures real values print correctly via implicit real-to-integer conversion.
@Scheremo Scheremo marked this pull request as ready for review October 21, 2025 11:38
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.

Holy cow, you can print reals with %d in Verilog 🤯? LGTM!

@Scheremo
Copy link
Contributor Author

Holy cow, you can print reals with %d in Verilog 🤯? LGTM!

You should've seen my face when I first figured that out, and then again when I figured out how wide that integer would have to be 🙈

@Scheremo Scheremo merged commit dc0a51b into llvm:main Oct 21, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants