Skip to content

[LTL] Make ltl.delay support optional clocking#9869

Open
Clo91eaf wants to merge 3 commits intollvm:mainfrom
Clo91eaf:explict-delay-1
Open

[LTL] Make ltl.delay support optional clocking#9869
Clo91eaf wants to merge 3 commits intollvm:mainfrom
Clo91eaf:explict-delay-1

Conversation

@Clo91eaf
Copy link
Contributor

@Clo91eaf Clo91eaf commented Mar 7, 2026

This PR is the first step of the explicit-delay stack.

Create a new operation of ltl.clocked_delay

Example:

ltl.delay %s, 2, 0
ltl.clocked_delay %clk, posedge, %s, 2, 0

This pr is participated with @Claude-opus-4.6 and reviewed by myself

@Clo91eaf Clo91eaf force-pushed the explict-delay-1 branch 2 times, most recently from c25092b to 2312c9d Compare March 7, 2026 04:01
@Clo91eaf
Copy link
Contributor Author

Clo91eaf commented Mar 7, 2026

@uenoku cc

Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

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

Probably we should add an operation instead of adding optional operand because we could break existing delay op's semantics.

@Clo91eaf
Copy link
Contributor Author

Clo91eaf commented Mar 7, 2026

Probably we should add an operation instead of adding optional operand because we could break existing delay op's semantics.

Ok, this way seems better, i will change the RFC first and then redo this pr's commits.

@Clo91eaf Clo91eaf marked this pull request as draft March 7, 2026 08:06
- Move ClockEdgeAttr and ClockOp definitions before the Sequences
  section so they can be referenced by sequence operations.
- Add new ClockedDelayOp for explicitly clocked delays with mandatory
  clock and edge parameters. Syntax: clocked_delay clock, edge, input, delay[, length].
- Simplify DelayOp to be purely unclocked (remove implicit clock support).
- Add ClockedDelayOp to LTL Visitor TypeSwitch for downstream consumers.
- Update DelayOp documentation to clarify unclocked semantics.
@Clo91eaf Clo91eaf force-pushed the explict-delay-1 branch 2 times, most recently from 206be23 to 8495815 Compare March 7, 2026 14:42
@Clo91eaf Clo91eaf marked this pull request as ready for review March 7, 2026 14:44
@Clo91eaf
Copy link
Contributor Author

Clo91eaf commented Mar 7, 2026

@uenoku cc

Clo91eaf added 2 commits March 7, 2026 23:00
- Add SameClockAndEdge constraint for matching clocked delay pairs.
- Add NestedClockedDelays pattern: merge nested clocked delays with
  same clock/edge into a single delay.
- Add MoveClockedDelayIntoConcat pattern for canonicalization.
- Add MergeNestedClockedDelays C++ canonicalization pattern.
- Use shared foldDelayLike<> template for both DelayOp and ClockedDelayOp.
- Document ClockedDelayOp syntax, semantics, and examples in LTL.md.
- Update DelayOp docs to clarify purely unclocked semantics.
- Add ClockedDelayOp roundtrip tests to basic.mlir.
- Update canonicalization.mlir for ClockedDelayOp fold patterns.
@Clo91eaf
Copy link
Contributor Author

@fabianschuiki cc pls 👀

@dobios
Copy link
Member

dobios commented Mar 10, 2026

I'm curious as to why this is totally necessary, won't this create semantic issues when paired with assert-property style things (so lowerings that yield clocked_assert) or other ltl.clock ops? Or are you going to add extra validation to enforce that only one clock is used across an expression like in the clocked assert validation?

@Clo91eaf
Copy link
Contributor Author

I'm curious as to why this is totally necessary

The initial issue came from #8673, and its main purpose was to ensure that IR (In-Relational Analysis) had clear semantic relationships when analyzing clocks.

won't this create semantic issues when paired with assert-property style things (so lowerings that yield clocked_assert) or other ltl.clock ops?

Good question. The current VerifyClockedAssertLike pass already walks the property tree under clocked_assert and rejects any nested ltl.clock ops. This PR (PR1) only adds the ClockedDelayOp definition — at this stage, nothing produces ClockedDelayOp yet, so there's no interaction to worry about.

In the follow-up PR (PR2: InferLTLClocks), ltl.clock ops are lowered by pushing their clock info into ltl.delayltl.clocked_delay, and then the ltl.clock is eliminated. So after InferLTLClocks runs, the clock lives exclusively on clocked_delay ops rather than on ltl.clock.

Or are you going to add extra validation to enforce that only one clock is used across an expression like in the clocked assert validation?

That said, you're right that VerifyClockedAssertLike should be updated to also reject ClockedDelayOp with mismatched clocks under clocked_assert. I'll add that check in PR2 — the validation will verify that all ClockedDelayOp clocks in the property tree are consistent with the clocked_assert clock, similar to the existing ltl.clock rejection.

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.

3 participants