[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