[PATCH] D80663: [LangRef] Fix description of shape args for matrix.multiply.
Florian Hahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 29 08:07:01 PDT 2020
fhahn added a comment.
In D80663#2060263 <https://reviews.llvm.org/D80663#2060263>, @braedy wrote:
> Theoretically, renaming the arguments from (M, N, K) to (M, K, N) doesn't break compatibility but updates the names to match BLAS, though the argument order no longer matches BLAS.
>
> If you'd prefer to rename with completely different names, I'm not sure there's a good name involving columns or rows for the middle dimension. My suggestion would be `outer1`, `outer2`, `inner`.
I think we should go with renaming to different names, to hopefully reduce the risk of further confusion. Something like `Outer1`, `Outer2`, `Inner` seems good, perhaps even `OuterRows`, `OuterColumns`?
In any case, if there aren't any concerns with updating LangRef until Monday or so I think we should submit the update then :)
For the title of the revision, I would suggest updating to something more descriptive, like `[LangRef] Fix description of shape args for matrix.multiply.`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80663/new/
https://reviews.llvm.org/D80663
More information about the llvm-commits
mailing list