[PATCH] D80663: Change description to match code representation.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

Thanks for raising this unfortunate inconsistency!

I think the interpretation used in the lowering is a bit more consistent with the interpretation of the shape arguments in the other intrinsics (which refer to the shape of the arguments directly), but the divergence from BLAS might indeed be a bit surprising. Perhaps using different names for `M`, `N`, `K` might be helpful to avoid confusion (e.g. something like `NumRows1`, `NumCols1`, `NumCols2`).

Changing the MatrixBuilder/LowerMatrixIntrinsics to use the different interpretation would indeed not be too much work in llvm-project, but it would break compatibility with modules constructed with the current MatrixBuilder. Given that code using the current interpretation in the langref would not work as expected anyways, I think we should try to preserve compatibility with working modules and update the LangRef. But it would be good to hear what other users think (all users of the intrinsics I am aware of should be on the review I think)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80663/new/

https://reviews.llvm.org/D80663