Initial plumbing for inner_tiled on CPU with data-tiled MMA attribute.#23494
Initial plumbing for inner_tiled on CPU with data-tiled MMA attribute.#23494bjacob merged 4 commits intoiree-org:mainfrom
Conversation
egebeysel
left a comment
egebeysel
left a comment
There was a problem hiding this comment.
Very interesting stuff so I self-appointed myself to review if you don't mind :) I don't really have much comments regarding the static cases - it LGTM from that front.
Though I'm actively working on scalable vector ISAs (SVE, SME, RVV) and have some concerns about the current semantics of the operation to model them. I left my thoughts as comments, but if I'm not mistaken - the operation currently does not allow them at all.
I'm willing to work on this and bolt in the missing pieces myself - I think it's important that we do this early rather than later after switching to use this op instead of the mmt4d. Though I'm not sure how that would play out with the current infrastructure built around the inner_tiled op. Let me know what you think!
Also, more of a question rather than a review, but what is the incentive of switching from mmt4d to inner_tiled for the CPU pipeline? Is it more to unify the data-tiling work for the CPU and GPU pipelines or is there an effective semantic gap between these op for the CPU?
|
Thanks @egebeysel for all the comments. We had a conversation offline and then here: https://discord.com/channels/689900678990135345/1456679894812590326/1474440363199434843 The summary is that we agree that further changes are needed anyway, but the present PR lays some groundwork that is useful regardless. I would like to go ahead and get this merged just to take it off of my stack of WIP PRs. |
krzysz00
left a comment
krzysz00
left a comment
There was a problem hiding this comment.
Interface implementations and such lgtm
egebeysel
left a comment
egebeysel
left a comment
There was a problem hiding this comment.
Thanks for the offline discussion. LGTM as well :)
hanhanW
left a comment
hanhanW
left a comment
There was a problem hiding this comment.
Just few nits. I don't know what the final picture is, but I'm sure that it is going to be great. I'm looking forward to it, and thanks for starting the work!
compiler/src/iree/compiler/Codegen/Dialect/CPU/IR/test/iree_cpu_inner_tiled_ops.mlir
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Dialect/CPU/IR/IREECPUEnums.h
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Dialect/CPU/IR/IREECPUEnums.td
Outdated
Show resolved
Hide resolved
Signed-off-by: Benoit Jacob <[email protected]>
Signed-off-by: Benoit Jacob <[email protected]>
Signed-off-by: Benoit Jacob <[email protected]>
Signed-off-by: Benoit Jacob <[email protected]>
4 participants
This is just the initial round of plumbing to get to the point where one can at least build and validate a
inner_tiledop on CPU, meaning with akindparameter that is a CPU-specific data-tiled MMA layout attr with an intrinsic enum that designates a SIMD intrinsic.Most of this was written by AI.