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

[TK] Add a TestLaunchContext for generating test dispatch IRs #429

Merged
merged 7 commits into from
Feb 13, 2024

Conversation

Groverkss
Copy link
Contributor

@Groverkss Groverkss commented Feb 12, 2024

This patch changes the way we do grid bindings. It is now expected that the IndexingContext will have the workload values and the grid on construction will try to build it's dims using them. Currently, all symbols are assumed to have a constant value, so the grid is able to map workgroup calculations to constants.

Ideally, we would have support to parameterize the kernel by some symbols, which would be passed as workload bindings (We have no support for this currently).

@Groverkss Groverkss marked this pull request as ready for review February 12, 2024 14:03
raise ValueError(
f"Cannot create {type(self)}({', '.join(str(i) for i in dims)}): mismatched symbolic rank"
)
def __init__(self):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of passing the grid bindings as inputs, I am now making the grid find out the symbol bindings from the IndexingContext, and if it cannot find a binding for the symbol, it should consider it a workload binding. Currently, this promotion to a workload binding is not implemented and every symbol is assumed to have a constant value.

self.rank = rank
self.rank = len(self.dims)

def __class_getitem__(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just to fix a type error. Metaclasses should not have class_getitem on them.

Copy link
Contributor

@stellaraccident stellaraccident left a comment

Choose a reason for hiding this comment

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

First pass: I need to get this review on a bigger screen for a more detailed look.

core/shark_turbine/kernel/_support/tracing.py Outdated Show resolved Hide resolved
Copy link
Contributor

@stellaraccident stellaraccident left a comment

Choose a reason for hiding this comment

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

LG. I think this is just about at the carrying capacity of what we can do without a pass through for more fine grained tests and code organization.

core/shark_turbine/kernel/_support/tracing.py Outdated Show resolved Hide resolved
core/shark_turbine/kernel/compiler/host_codegen.py Outdated Show resolved Hide resolved
@Groverkss Groverkss merged commit e955627 into nod-ai:main Feb 13, 2024
4 checks passed
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