-
Notifications
You must be signed in to change notification settings - Fork 802
Change the query API to match the new changes made to the matrix API #6981
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
Conversation
|
ping @yubingex007-a11y |
'intel_llvm/sycl' into matrix-query X
| matrix_type atype; | ||
| matrix_type btype; | ||
| matrix_type ctype; | ||
| matrix_type acuumulatortype; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accumulator type
| @@ -0,0 +1,143 @@ | |||
| // RUN: %clangxx -DSYCL_EXT_ONEAPI_MATRIX_VERSION=2 -fsycl -o query-use %s | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we have a testcase in llvm-test-suite and check the query result?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yubingex007-a11y
I am adding a test now.
BTW, https://github.com/intel/llvm/blob/sycl/sycl/include/sycl/ext/oneapi/matrix/static-query-use.hpp#L154 is currently wrong because in matrix-jit-use, we assume now that layout for A, B and C are unused.
I am correcting this now. But with adapting to the new API that is now merged (https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/experimental/sycl_ext_oneapi_matrix/sycl_ext_oneapi_matrix.asciidoc), we need to change this back because in the new (portable) API, layout on A and B is required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we have a testcase in llvm-test-suite and check the query result?
intel/llvm-test-suite#1358 adds the test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ping @yubingex007-a11y
|
@againull, can you please help review/merge? |
|
@intel/llvm-reviewers-runtime, can you please help merge this? |
#6662