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.

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

split convert_stream_to_snax_stream in scheduler and layout resolution #332

Merged
merged 3 commits into from
Jan 6, 2025

Conversation

jorendumoulin
Copy link
Contributor

stacked on #331

With the addition of the schedule op, this PR splits the convert_stream_to_snax_stream (which converts operation -> access pattern) into two separate passes (scheduler: operation -> schedule), (layout resolution: schedule -> access pattern).

The schedule representation is nice to have to run other passes on before doing layout resolution, such as selecting a layout.

Copy link
Contributor

@JosseVanDelm JosseVanDelm left a comment

Choose a reason for hiding this comment

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

I have some small organizational proposed changes.
accfg registry is definitly a bit messy in this regard, which is a shame, but also no deal-breaker right now.
Can you also maybe update the comments a bit?
I guess more changes are also coming in future PRs, so please do when you see fit!



@dataclass
class MemrefStreamToSnaxPattern(RewritePattern):
class AutoflowScheduler(RewritePattern):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update the documentation here?

Comment on lines +34 to +38
# get template and template_bounds
accelerator_type = AcceleratorRegistry().get_acc_info(acc_op)
assert issubclass(accelerator_type, SNAXStreamer)

template = accelerator_type.get_template(op)
Copy link
Contributor

Choose a reason for hiding this comment

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

Excited to move this into the IR at some point!

from compiler.ir.stream.access_pattern import Template


def get_accelerator_info(op: stream.StreamingRegionOpBase) -> Template:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def get_accelerator_info(op: stream.StreamingRegionOpBase) -> Template:
def get_accelerator_template(op: stream.StreamingRegionOpBase) -> Template:

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make this a method of the (base class of the) op itself maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unfortunately this would be a circular import

class LayoutResolution(RewritePattern):
@op_type_rewrite_pattern
def match_and_rewrite(self, op: stream.ScheduleOp, rewriter: PatternRewriter):
template = get_accelerator_info(op)
Copy link
Contributor

Choose a reason for hiding this comment

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

If this were a method:

Suggested change
template = get_accelerator_info(op)
template = op.get_accelerator_template()

Base automatically changed from joren/add-schedule-op to main January 6, 2025 12:41
@jorendumoulin jorendumoulin merged commit c1f15ad into main Jan 6, 2025
14 checks passed
@jorendumoulin jorendumoulin deleted the joren/split-pass branch January 6, 2025 13:00
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