Skip to content

Allow passing locks to C code. #288

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

Open
stephenneuendorffer wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Allow passing locks to C code. #288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions test/unit_tests/chess_compiler_tests/01_precompiled_core_function/aie.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ module @test_chesss_01_precompiled_core_function {
%lock13_3 = AIE.lock(%tile13, 3) { sym_name = "input_lock" }
%lock13_5 = AIE.lock(%tile13, 5) { sym_name = "output_lock" }

func.func private @func(%A: memref<256xi32>, %B: memref<256xi32>) -> ()
func.func private @func(%A: memref<256xi32>, %B: memref<256xi32>, %lock: index) -> ()
Copy link
Contributor

@keryell keryell Apr 28, 2023

Choose a reason for hiding this comment

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

Perhaps index could be a more precise type depending on the architecture?
Or at least lowered to a more precise concrete type before calling the real AIE kernel?


%core13 = AIE.core(%tile13) {
AIE.useLock(%lock13_3, "Acquire", 1) // acquire for read(e.g. input ping)
AIE.useLock(%lock13_5, "Acquire", 0) // acquire for write
func.call @func(%buf13_0, %buf13_1) : (memref<256xi32>, memref<256xi32>) -> ()
func.call @func(%buf13_0, %buf13_1, %lock13_3) : (memref<256xi32>, memref<256xi32>, index) -> ()
AIE.useLock(%lock13_3, "Release", 0) // release for write
AIE.useLock(%lock13_5, "Release", 1) // release for read
AIE.end
Expand Down
4 changes: 3 additions & 1 deletion test/unit_tests/chess_compiler_tests/01_precompiled_core_function/kernel.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@

#include "kernel.h"

void func(int32_t *a, int32_t *b)
void func(int32_t *a, int32_t *b, int64_t lock)
Copy link
Contributor

@keryell keryell Apr 28, 2023

Choose a reason for hiding this comment

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

Bad for the carbon footprint to have 64 bits here?

Copy link
Collaborator Author

@stephenneuendorffer stephenneuendorffer Apr 28, 2023

Choose a reason for hiding this comment

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

Yes.. this is the main reason why this hasn't been checked in. Today, it's relatively easy to lower from locks to a constant 'index' type in MLIR, but the size of the index type is something 'implementation defined' (by default 64 bits). This really should be cleaner in the lowering to result in something more convenient.

{
acquire(lock, 1);
int val=a[3];
int val2=val+val;
val2 += val;
val2 += val;
val2 += val;
b[5] = val2;
release(lock, 0);
}

2 changes: 1 addition & 1 deletion test/unit_tests/chess_compiler_tests/01_precompiled_core_function/kernel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@
#include <stdio.h>

extern "C" {
void func(int32_t *a, int32_t *b);
void func(int32_t *a, int32_t *b, int64_t lock);
}