[PATCH] D81472: [Matrix] Update load/store intrinsics.

Braedy Kuzma via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 9 10:25:31 PDT 2020


braedy added a comment.

I admittedly don't know Clang's naming scheme, so feel free to ignore this, but I dislike the change to `*.column.major.*` because it feels like both are changeable parameters (i.e. column/row major/minor are two different axes). To anyone who knows what they're looking for they would understand that this doesn't make any sense, but the potential remains. I'm not sure that dropping the major portion makes any sense either because it might seem like you're promising loads and stores no matter the storage order (e.g. `llvm.matrix.column.load` might seem like it promises a column load even if the matrix is row major, which isn't the intent). Removing the `.` between them is an option but then it's not that much different from the original `columnwise`, unless "in line with Clang's naming" means using "column major" instead of "column-wise". Maybe I'm just putting too much meaning into the `.` too, but I'd rather mention it and be told it's fine than not say anything :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81472





More information about the llvm-commits mailing list