This patch adds MLIR NVVM support for the various NVPTX mma.sync
operations. There are a number of possible data type, shape,
and other attribute combinations supported by the operation, so a
custom assebmly format is added and attributes are inferred where
possible.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Looks good to me overall. Added some minor comments.
mlir/include/mlir/Dialect/LLVMIR/NVVMOps.td | ||
---|---|---|
928 | Is there any advantage having this optional? Always having it looks more consistent to me. | |
mlir/lib/Dialect/LLVMIR/IR/NVVMDialect.cpp | ||
73 | I would remove the static and add a some basic doc explaining what the function does, | |
115 | nit: missing space. | |
181 | I'd remove that, I don't see it anywhere else in mlir | |
377 | remove? |
Address comments and add missing f64 mma cases.
mlir/include/mlir/Dialect/LLVMIR/NVVMOps.td | ||
---|---|---|
928 | I could make it non-optional, but it would increase the verbosity in the case where the two attributes can be easily inferred (for all types above s8/u8). | |
mlir/lib/Dialect/LLVMIR/IR/NVVMDialect.cpp | ||
181 | The problem is that if you try to build this op using the default builders, it's a pain because of all the details. I'm using it in lowering pathways I've implemented already, but I didn't see an easy place to insert a unit test for builders like this | |
377 | Yes and it looks like I'm missing f64 tests/support below for m=8, I'll fix that |
Is there any advantage having this optional? Always having it looks more consistent to me.