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

Add normalizer tests for @dim attributes. #14

Draft
wants to merge 5 commits into
base: devel
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 16 additions & 0 deletions tests/functional/configs/test_suite_normalize/dim_normalize.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[
{
"action": "normalizer",
"action_config": {
"source": "normalize/dim/attr_placement.cpp"
},
"reference": "normalize/dim/attr_placement_ref.cpp"
},
{
"action": "normalizer",
"action_config": {
"source": "normalize/dim/attr_use.cpp"
},
"reference": "normalize/dim/attr_use_ref.cpp"
}
]
1 change: 1 addition & 0 deletions tests/functional/configs/test_suite_normalize/suite.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[
"simple_normalize.json",
"dim_normalize.json",
"directive.json",
"macro_expansion.json",
"atomic_test_case.json",
Expand Down
21 changes: 21 additions & 0 deletions tests/functional/data/normalize/dim/attr_placement.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
typedef float* mat4 @dim(4, 4);

@kernel void addVectors(const int entries, @dim(x,y) const float *a, const float *b @dim(y,x), float *ab) {
@tile(4, @outer, @inner)
for (int i = 0; i < 4; ++i) {
// Single
{
mat4 @dimOrder(1, 0) arr1 = ab;
arr1(1, 1) = 0;
int @dim(x,y) arr2[12];
int arr3[12] @dim(x,y) = { 0 };
};

// Multiple
{
@dim(x,y) int arr1_1[12], arr1_2[12];
int @dim(x,y) arr2_1[12], arr2_2[12];
int arr3_1[12] @dim(x,y), arr3_2[12] @dim(y,x);
};
};
}
23 changes: 23 additions & 0 deletions tests/functional/data/normalize/dim/attr_placement_ref.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
typedef [[okl::dim("(4,4)")]] float *mat4;
Copy link
Collaborator Author

@kchabSS kchabSS Jan 31, 2024

Choose a reason for hiding this comment

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

It is expected that type attributes are added just before the type itself by default

Copy link
Collaborator

Choose a reason for hiding this comment

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

One of the main purpose of normalizer is to unify using of attribute in terms of location.

[[okl::dim("(4,4)")]] typedef float *mat4;

is completely valid syntax without changing semantic.

Multivar declarations is the corner case and is broken by design regarding standard attributes.
To address this special case the multivar will be splitted into single var using clang-tidy with readability-isolate-declaration fixit.

Copy link
Collaborator Author

@kchabSS kchabSS Feb 1, 2024

Choose a reason for hiding this comment

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

One of the main purpose of normalizer is to unify using of attribute in terms of location.

[[okl::dim("(4,4)")]] typedef float *mat4;

is completely valid syntax without changing semantic.

Multivar declarations is the corner case and is broken by design regarding standard attributes. To address this special case the multivar will be splitted into single var using clang-tidy with readability-isolate-declaration fixit.

The above statement is totally wrong.
typedefs can define multiple 'aliases' to the same base type.
By definition, they are part of one.

Here we have a few solutions:

  • Write attribute before the type: typedef [[]] int A, *B
  • Write attribute(s) before or after the declaration: typedef int A [[]], *B [[]]
  • Do not change attribute position: typedef [[]] int A, *B

The second one duplicates the attribute changing it's nature from type attribute to decl attribute.
The last one works fine for most cases because we treat special case attributes as type attributes anyway.


[[okl::kernel("(void)")]] void addVectors(const int entries,
[[okl::dim("(x,y)")]] const float *a,
[[okl::dim("(y,x)")]] const float *b,
float *ab) {
[[okl::tile("(4,@outer,@inner)")]] for (int i = 0; i < 4; ++i) {
// Single
{
[[okl::dimOrder("(1,0)")]] mat4 arr1 = ab;
arr1(1, 1) = 0;
[[okl::dim("(x,y)")]] int arr2[12];
[[okl::dim("(x,y)")]] int arr3[12] = {0};
};

// Multiple
{
[[okl::dim("(x,y)")]] int arr1_1[12], arr1_2[12];
Copy link
Collaborator Author

@kchabSS kchabSS Jan 31, 2024

Choose a reason for hiding this comment

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

If the same attribute applied to both variables, it must be positioned at the beginning before type.

int [[okl::dim("(x,y)")]] arr2_1[12], arr2_2[12];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Attribute type must be applied only to the first definition.

int [[okl::dim("(x,y)")]] arr3_1[12], [[okl::dim("(y,x)")]] arr3_2[12];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Two different attributes on single multi-definition.
Each attribute must be just shy before the variable name.

};
};
}
23 changes: 23 additions & 0 deletions tests/functional/data/normalize/dim/attr_use.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
typedef int* iPtr45 @dim(4, 5);
typedef int iMat455[4*5*5] @dim(4, 5, 5);

struct sMat24 {
int* a @dim(2, 4);
};

@kernel void test(iPtr45 a @dimOrder(1, 0), sMat24 *b, iMat455 &ab @dimOrder(2, 1, 0), float *ac @dim(4,5) @dimOrder(0, 1)) {
for (int i = 0; i < 4; ++i; @outer) {
@shared int cc[5*4] @dim(4, 5) @dimOrder(0, 1);
for (int j = 0; j < 5; ++j; @inner) {
cc(j, i) = 0;

for (int k = 0; k < j; ++k) {
ab(k, j, i) = a(j, i);
ab(k, j, i) += b->a(i, j);
}

ac(j, i) = a(j, i);
ac(j, i) += b->a(i, j);
}
}
}
26 changes: 26 additions & 0 deletions tests/functional/data/normalize/dim/attr_use_ref.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
typedef [[okl::dim("(4,5)")]] int *iPtr45;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Type attribute must be placed before the type itself.
OR before the variable/type name in case of multiple variable/type definitions per single declaration.

typedef [[okl::dim("(4,5,5)")]] int iMat455[4 * 5 * 5];

struct sMat24 {
[[okl::dim("(2,4)")]] int *a;
};

[[okl::kernel("(void)")]] void
test([[okl::dimOrder("(1,0)")]] iPtr45 a, sMat24 *b,
[[okl::dimOrder("(2,1,0)")]] iMat455 &ab,
[[okl::dim("(4,5)")]] [[okl::dimOrder("(0,1)")]] float *ac) {
[[okl::outer("(void)")]] for (int i = 0; i < 4; ++i) {
[[okl::shared("(void)")]] [[okl::dim("(4,5)")]] [[okl::dimOrder("(0,1)")]] int cc[5 * 4];
[[okl::inner("(void)")]] for (int j = 0; j < 5; ++j) {
cc(j, i) = 0;

for (int k = 0; k < j; ++k) {
ab(k, j, i) = a(j, i);
ab(k, j, i) += b->a(i, j);
}

ac(j, i) = a(j, i);
ac(j, i) += b->a(i, j);
}
}
}