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

[Optimizer] Simple shard layout picker #1150

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

nobradovictt
Copy link
Contributor

Add shard spec constraint within ShardSolver - match shard memory layout on input and output.
Fix height and width shard spec generation.
Add simple shard layout picker.

MNIST is now automatically sharded without overrides!

Closes #1147

Copy link
Contributor

@odjuricicTT odjuricicTT left a comment

Choose a reason for hiding this comment

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

Yeeeees!

@nobradovictt nobradovictt force-pushed the nobradovic/sharding_constraints branch from 1980a6c to b452c61 Compare November 5, 2024 13:31
@nobradovictt nobradovictt enabled auto-merge (squash) November 5, 2024 13:32
@@ -48,6 +48,19 @@ def TT_GridAttr : TT_Attr<"Grid", "grid"> {
static GridAttr get(::mlir::MLIRContext *context, std::int64_t rank) {
return GridAttr::get(context, SmallVector<std::int64_t>(rank, 1));
}

uint64_t mutable cNumUsedCores = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

mutable is used for const attributes, does tablegen create this var as a const?

Copy link
Contributor Author

@nobradovictt nobradovictt Nov 5, 2024

Choose a reason for hiding this comment

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

Util getter I made is const func and this is used as a cache for result of the getter. I might have gone overboard on this one, but I didn't want to repeat this multiplication for every time getNumUsedCores is invoked.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. I'd preferably call this gridVolume.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was in auto merge mode and didn't stop it, I can rename it in my next change.

@nobradovictt nobradovictt merged commit a38cc9d into main Nov 5, 2024
17 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.

Introduce loose sharding constraints in Shardsolver
4 participants