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

Support tt-metal semaphores #919

Merged
merged 4 commits into from
Jan 22, 2025
Merged

Conversation

pjanevskiTT
Copy link
Contributor

@pjanevskiTT pjanevskiTT commented Oct 15, 2024

Fix #915

Add support for all tt-metal semaphore functions, as well as some utility functions that are useful for using semaphores through compiler

For example, code below

Value zero = i32(0, builder);
auto sem_id = builder.create<ttkernel::GetArgValOp>(arithOrMathOp.getLoc(), zero);
auto sem_addr = builder.create<ttkernel::GetSemaphoreOp>(arithOrMathOp.getLoc(), sem_id);
auto sem_l1_ptr = builder.create<ttkernel::CastToL1PtrOp>(arithOrMathOp.getLoc(), sem_addr);
builder.create<ttkernel::NocSemaphoreWaitOp>(arithOrMathOp.getLoc(), sem_l1_ptr, zero);

will generate kernel code

int32_t v5 = get_arg_val<uint32_t>(v4);
int32_t v6 = get_semaphore(v5);
volatile tt_l1_ptr uint32_t*  v7 = reinterpret_cast<volatile tt_l1_ptr uint32_t*>(v6);
noc_semaphore_wait(v7, v4);

TODO

  • Implement conversion to tt_l1_ptr properly

@pjanevskiTT pjanevskiTT force-pushed the pjanevski/metal_direct_semaphore branch from ce53db7 to 7de191f Compare October 15, 2024 15:25
@pjanevskiTT pjanevskiTT force-pushed the pjanevski/metal_direct_semaphore branch from 7de191f to 58e1fae Compare October 15, 2024 15:43
@pjanevskiTT pjanevskiTT marked this pull request as ready for review October 15, 2024 15:51
Copy link
Contributor

@nsmithtt nsmithtt left a comment

Choose a reason for hiding this comment

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

This looks great! Minor comments inline

lib/Dialect/TTMetal/IR/TTMetalOps.cpp Outdated Show resolved Hide resolved
include/ttmlir/Dialect/TTKernel/IR/TTKernelOps.td Outdated Show resolved Hide resolved
include/ttmlir/Dialect/TTKernel/IR/TTKernelOps.td Outdated Show resolved Hide resolved
lib/Dialect/TTMetal/IR/TTMetalOps.cpp Outdated Show resolved Hide resolved
@@ -187,6 +199,18 @@ class TTMetalToEmitCOpaqueRewriter : public OpConversionPattern<SourceOp> {
return name;
}

ArrayAttr getTemplateArgs(SourceOp op) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

I have addded similar function for reduce template args, I guess you didn't rebase yet.

@rpavlovicTT
Copy link
Contributor

Nice changes, few small comments, but overall looks good.

@jdesousa-TT jdesousa-TT force-pushed the pjanevski/metal_direct_semaphore branch 3 times, most recently from 61de687 to 28b9d4a Compare January 21, 2025 16:17
@jdesousa-TT jdesousa-TT requested a review from nsmithtt January 21, 2025 16:18
@jdesousa-TT
Copy link
Contributor

jdesousa-TT commented Jan 21, 2025

@nsmithtt / @xanderchin with regard to #915, please re-review. The existing comments have been addressed, and this branch rebased. Ready for merge.

@jdesousa-TT jdesousa-TT force-pushed the pjanevski/metal_direct_semaphore branch from 28b9d4a to 627222a Compare January 21, 2025 16:20
Copy link
Contributor

@nsmithtt nsmithtt left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @jdesousa-TT for revitalizing this one.

@jdesousa-TT jdesousa-TT merged commit 4ba1b73 into main Jan 22, 2025
23 checks passed
@jdesousa-TT jdesousa-TT deleted the pjanevski/metal_direct_semaphore branch January 22, 2025 15:49
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.

[Metal Direct] Add suport for semaphores
4 participants