Skip to content

[Synth] Add declarative cut rewrite pattern and yield op#10565

Open
okekayode wants to merge 4 commits into
llvm:mainfrom
okekayode:dev/add-op-for-declarative-cut-rewrite
Open

[Synth] Add declarative cut rewrite pattern and yield op#10565
okekayode wants to merge 4 commits into
llvm:mainfrom
okekayode:dev/add-op-for-declarative-cut-rewrite

Conversation

@okekayode
Copy link
Copy Markdown
Contributor

@okekayode okekayode commented May 31, 2026

Concerns #10485

We introduce the declarative operation synth.cut_rewrite_pattern and also the terminator operation synth.yield.

synth.cut_rewrite_pattern denotes cut rewrite patterns for i1 input and result types as function-shaped regions. We also implement a custom printer and parser with attribute support for synth.mapping_cost.

In this PR we only introduce the IR representation; wiring this into consumers, for example TechMapper.cpp, can be added later.

@okekayode okekayode changed the title [Synth] Add an operation for declarative Cut rewrite pattern [Synth] Add declarative cut rewrite pattern and yield op May 31, 2026
@circt-bot
Copy link
Copy Markdown

circt-bot Bot commented May 31, 2026

Results of circt-tests run for 833947f compared to results for 7aa41cc: no change to test results.

Comment thread include/circt/Dialect/Synth/SynthOps.td Outdated
@okekayode okekayode force-pushed the dev/add-op-for-declarative-cut-rewrite branch 2 times, most recently from 5ba04fa to 59ffecd Compare May 31, 2026 05:56
Comment thread lib/Dialect/Synth/SynthOps.cpp Outdated
Comment thread lib/Dialect/Synth/SynthOps.cpp Outdated
@okekayode okekayode force-pushed the dev/add-op-for-declarative-cut-rewrite branch from 59ffecd to aee47a9 Compare May 31, 2026 06:15
@circt-bot
Copy link
Copy Markdown

circt-bot Bot commented May 31, 2026

Results of circt-tests run for aee47a9 compared to results for 7aa41cc: no change to test results.

Comment thread lib/Dialect/Synth/SynthOps.cpp Outdated
Comment thread lib/Dialect/Synth/SynthOps.cpp Outdated
Comment thread lib/Dialect/Synth/SynthOps.cpp Outdated
@okekayode okekayode force-pushed the dev/add-op-for-declarative-cut-rewrite branch from aee47a9 to 2318e6e Compare May 31, 2026 07:51
@circt-bot
Copy link
Copy Markdown

circt-bot Bot commented May 31, 2026

Results of circt-tests run for 2318e6e compared to results for 7aa41cc: no change to test results.

Comment thread test/Dialect/Synth/round-trip.mlir Outdated
Comment thread lib/Dialect/Synth/SynthOps.cpp Outdated
Copy link
Copy Markdown
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.

LGTM

@okekayode okekayode force-pushed the dev/add-op-for-declarative-cut-rewrite branch from 2318e6e to 030d0a0 Compare May 31, 2026 19:12
@circt-bot
Copy link
Copy Markdown

circt-bot Bot commented May 31, 2026

Results of circt-tests run for 030d0a0 compared to results for 7aa41cc: no change to test results.

@okekayode okekayode force-pushed the dev/add-op-for-declarative-cut-rewrite branch from 030d0a0 to 8e7d483 Compare June 2, 2026 18:07
Comment thread lib/Dialect/Synth/SynthOps.cpp Outdated

auto cost = getCost();
if (auto arcs = cost.getArcs())
if (!arcs.empty())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry I'm not sure I'm following this. I do expect CutRewriterPattern to use delay from arc attributes. Is it included in your plan to support this by updating MappingCostAttr?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added that check since the current arc representation is name-based, so non-empty arcs would suggest input/output names.

I see your confusion: this is not the intended final behaviour. My plan would be to support positional arcs in MappingCostAttr (via Dictionary form), or add another synth positional arc attribute that MappingCostAttr can contain (i think this is cleaner), and then validate those arcs in CutRewritePatternOp::verify() against the function input/result counts.

My bad, sorry for the confusion.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's ok to reject this if there is a plan, but please add TODO comments. Also I don't think we want to have two different linear timing arc attributes, so could you revert the new attribute? I think we can keep using the LinearTimingArcAttr by simply ignoring names (maybe in the future we also want to drop names eventually).

Also MappingCostAttr could use 2d array attributes as a container for arc, I believ,.

@circt-bot
Copy link
Copy Markdown

circt-bot Bot commented Jun 2, 2026

Results of circt-tests run for 8e7d483 compared to results for 7aa41cc: no change to test results.

@okekayode okekayode force-pushed the dev/add-op-for-declarative-cut-rewrite branch from 08f5c93 to a4a11d2 Compare June 2, 2026 22:22
@circt-bot
Copy link
Copy Markdown

circt-bot Bot commented Jun 2, 2026

Results of circt-tests run for a4a11d2 compared to results for 7aa41cc: no change to test results.

@circt-bot
Copy link
Copy Markdown

circt-bot Bot commented Jun 3, 2026

Results of circt-tests run for 62d70f5 compared to results for 7aa41cc: no change to test results.

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.

2 participants