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

Allow passing locks to C code. #288

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

Conversation

stephenneuendorffer
Copy link
Collaborator

No description provided.

@@ -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
Member

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

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.

@@ -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
Member

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?

@keryell
Copy link
Member

keryell commented Jul 20, 2023

To be merged?

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