Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

dialects: (scf) Allow implicit terminator (yield op) in scf.for operation #1607

Merged
merged 9 commits into from
Oct 30, 2023
Merged

dialects: (scf) Allow implicit terminator (yield op) in scf.for operation #1607

merged 9 commits into from
Oct 30, 2023

Conversation

compor
Copy link
Collaborator

@compor compor commented Sep 25, 2023 •
edited
Loading

This PR adds:

  • Automatic generation of the scf.yield terminator in scf.for operations that have no loop-carried variables via the ensure_terminator method (only for custom printing/parsing).
  • Skipping printing of scf.yield for scf.for operations if it has no loop-carried variables (only for custom printing/parsing).
  • Test case for the above (triple scf.for loop nest taken from MLIR test suite).

TODO:

Once these are done I'll update and remove draft status of this PR, I just wanted to have some eyes on it first.

Downsizing tasks due to lack of time ATM (just need to push this).

@compor compor added dialects Changes on the dialects core xDSL core (ir, textual format, ...) labels Sep 25, 2023
@compor compor self-assigned this Sep 25, 2023
@compor compor marked this pull request as draft September 25, 2023 14:45
compor
compor commented Sep 25, 2023
View reviewed changes
tests/test_printer.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 25, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Files Coverage Δ
tests/filecheck/frontend/dialects/scf.py 50.57% <ø> (ø)
xdsl/dialects/scf.py 92.33% <100.00%> (+0.03%) ⬆️

📢 Thoughts on this report? Let us know!.

PapyChacal
PapyChacal reviewed Sep 25, 2023
View reviewed changes
xdsl/printer.py Outdated Show resolved Hide resolved
@compor compor force-pushed the christos/core/implicit-term branch from 918ee2c to be7039f Compare September 27, 2023 13:25
@compor compor mentioned this pull request Sep 28, 2023
Merged
compor added a commit that referenced this pull request Sep 29, 2023
@compor
core: (printer) Allow optional printing of block terminator operations (
c779736 
#1617)

This PR is part of #1607 and adds the ability to optionally skip printing a terminator operation.

When this is to happen is left to the specific dialect; the immediate
application of this is used in custom printing/parsing in order to avoid
printing implicit terminators.
@compor compor changed the title [WIP] dialects: (scf) Allow implicit terminator (yield op) in scf operations [WIP] dialects: (scf) Allow implicit terminator (yield op) in scf.for operation Oct 26, 2023
@compor compor marked this pull request as ready for review October 26, 2023 12:14
@compor compor changed the title [WIP] dialects: (scf) Allow implicit terminator (yield op) in scf.for operation dialects: (scf) Allow implicit terminator (yield op) in scf.for operation Oct 26, 2023
@compor compor requested a review from AntonLydike October 26, 2023 12:26
@compor compor requested a review from PapyChacal October 27, 2023 13:39
AntonLydike
AntonLydike approved these changes Oct 30, 2023
View reviewed changes
Copy link
Collaborator

@AntonLydike AntonLydike left a comment

Choose a reason for hiding this comment

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

👍

@compor
Copy link
Collaborator Author

compor commented Oct 30, 2023

@math-fehr ?

math-fehr
math-fehr approved these changes Oct 30, 2023
View reviewed changes
Copy link
Collaborator

@math-fehr math-fehr left a comment

Choose a reason for hiding this comment

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

Nice!

@compor compor merged commit 9fff88c into main Oct 30, 2023
@compor compor deleted the christos/core/implicit-term branch October 30, 2023 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@math-fehr math-fehr math-fehr approved these changes

@AntonLydike AntonLydike AntonLydike approved these changes

@superlopuh superlopuh superlopuh approved these changes

@webmiche webmiche Awaiting requested review from webmiche

@PapyChacal PapyChacal Awaiting requested review from PapyChacal

Assignees

@compor compor

Labels
core xDSL core (ir, textual format, ...) dialects Changes on the dialects
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@compor @superlopuh @AntonLydike @math-fehr @PapyChacal