[PATCH] D80321: [mlir][spirv] Enable composite instructions for cooperative matrix type.

[Lei Zhang via Phabricator via llvm-commits]

antiagainst requested changes to this revision.
antiagainst added inline comments.
This revision now requires changes to proceed.


================
Comment at: mlir/include/mlir/Dialect/SPIRV/SPIRVTypes.h:143
+  /// implementation dependent.
+  bool hasDynamicNumElements() const;
+
----------------
Could you add a comment to `getNumElements` to mention that this method being false is a pre-condition? (With that actually it would be nice to rename this as `hasCompileTimeKnownNumElements` or something so that we don't always use it as the negate form. But a minor issue.)


================
Comment at: mlir/test/Dialect/SPIRV/Serialization/cooperative-matrix.mlir:100
+    %1 = spv.AccessChain %a[%0] : !spv.ptr<!spv.coopmatrix<8x16xf32, Subgroup>, StorageBuffer>
+    spv.Return
+  }
----------------
Can we return `%1` here so we can check the return type too? Right now it's hidden. 


================
Comment at: mlir/test/Dialect/SPIRV/composite-ops.mlir:25
+  // CHECK: spv.CompositeConstruct {{%.*}}, {{%.*}}, {{%.*}}, {{%.*}} : !spv.coopmatrix<8x16xf32, Subgroup>
+  %0 = spv.CompositeConstruct %arg0, %arg1, %arg2, %arg3 : !spv.coopmatrix<8x16xf32, Subgroup>
+  return %0: !spv.coopmatrix<8x16xf32, Subgroup>
----------------
I think this should only take one operand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80321