Skip to content

Conversation

@fabianschuiki
Copy link
Contributor

Add support for lowering the moore.builtin.time op to a new llhd.current_time op that simply materializes the current simulation time as an SSA value.

Error diff on circt-tests:

-37 error: failed to legalize operation 'moore.builtin.time'
+22 error: failed to legalize operation 'moore.time_to_logic'
+15 error: failed to legalize operation 'moore.fmt.time'
  0 total change

Copy link
Member

@maerhart maerhart left a comment

Choose a reason for hiding this comment

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

LGTM

let hasFolder = 1;
}

def CurrentTimeOp : LLHDOp<"current_time", [Pure]> {
Copy link
Member

Choose a reason for hiding this comment

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

It would be illegal to move this operation across an llhd.wait operation, right? E.g., by CSE. So, it shouldn't be marked Pure. Maybe as Read?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch, thanks! 💯 I've marked this as having a read side-effect instead, and there are new tests to check that unused ops can be DCEd, and that ops only CSE if there is no side-effecting op in between.

Add support for lowering the `moore.builtin.time` op to a new
`llhd.current_time` op that simply materializes the current simulation
time as an SSA value.

Error diff on circt-tests:
```diff
-37 error: failed to legalize operation 'moore.builtin.time'
+22 error: failed to legalize operation 'moore.time_to_logic'
+15 error: failed to legalize operation 'moore.fmt.time'
  0 total change
```
@fabianschuiki fabianschuiki merged commit 7182b35 into main Dec 22, 2025
7 checks passed
@fabianschuiki fabianschuiki deleted the fschuiki/current-time branch December 22, 2025 19:52
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.

3 participants