-
Notifications
You must be signed in to change notification settings - Fork 612
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
[Codegen][CPU] Enable scalable transfer lowerings #18170
Conversation
09a58cd
to
b28f396
Compare
This enables a general scalable lowering for `transfer_write(transpose)` when ArmSME is _not_ available. The ArmSME dialect already had its own (more specific) lowerings for cases like this, which is why these lowerings are disabled when SME is available. Depends on: llvm/llvm-project#101353 Signed-off-by: Benjamin Maxwell <[email protected]>
Signed-off-by: Benjamin Maxwell <[email protected]>
b28f396
to
645ab7d
Compare
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.
LGTM cheers
// CHECK-LABEL: func.func @scalable_transpose_store | ||
// CHECK-NOT: vector.transpose | ||
// CHECK: vector.store {{.*}} : memref<?x?xf32>, vector<4xf32> | ||
// CHECK-NOT: vector.transpose |
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.
Why do we need the second CHECK-NOT
?
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.
CHECK-NOT
only checks between two matches. The first checks between func.func @scalable_transpose_store
and vector.store
, the second checks from vector.store
to the end of the function (IIRC).
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.
Yeah, I understand what it is happening. My point is that the check of vector.store
already shows that the lowering happens. In this case, why do we need to check if there is a vector.transpose
followed by it?
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.
I also want to check that the transpose (which is not directly supported) is eliminated.
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.
LGTM, just one question about the test.
This enables a general scalable lowering for
transfer_write(transpose)
when ArmSME is not available. The ArmSME dialect already had its own (more specific) lowerings for cases like this, which is why these lowerings are disabled when SME is available.Depends on: llvm/llvm-project#101353