[PATCH] D80181: [mlir][spirv] Add remaining cooperative matrix instructions.

[Thomas Raoux via Phabricator via llvm-commits]

ThomasRaoux marked 8 inline comments as done.
ThomasRaoux added inline comments.


================
Comment at: mlir/include/mlir/Dialect/SPIRV/SPIRVCooperativeMatrixOps.td:56
+  );
+  let verifier = [{ return success(); }];
+}
----------------
antiagainst wrote:
> I think we can add 
> 
> ```
> let assemblyFormat = "attr-dict `:` type($result)";
> ```
> 
> So that we don't need to manually write the parser and printer?
Here type is not the type of $result so this syntax wouldn't work. Note sure if there is a way to get it to pick up type for the argument?


================
Comment at: mlir/include/mlir/Dialect/SPIRV/SPIRVCooperativeMatrixOps.td:133
 
   let verifier = [{ return success(); }];
 }
----------------
antiagainst wrote:
> I think we need verification on `$pointer`. At the moment every pointer will be accepted.
I added some verification for both load and store pointer type however I didn't find an easy way to test the verification as the invalid IR would fail parsing.


================
Comment at: mlir/include/mlir/Dialect/SPIRV/SPIRVCooperativeMatrixOps.td:176
+                              sssa-use `,` ssa-use `,` ssa-use ` : `
+                              cooperative-matrix-type
+    ```
----------------
antiagainst wrote:
> One general rule in MLIR regarding assembly is that it should be parsable on its own. I think we need to give at least two types (for `$a` and `$b`) for this? Otherwise by only looking at this op, I'm not able to tell what the type is for `$a` and `$b`.
My bad I had wrongly assumed the type were matching. I added 2 types and a result type even though the result type could be deducted from the operand types.
I wasn't able to get the assemblyFormat tow work as I could find a way to make the type of c the same as type of result. (type($c, $result) doesn't work) I haven't look very deeply though I can investigate more if you want. Fow now I left the custom parsing/printing functions.

I also added a verify function for muladd and some tests along with it. I was planning to add those later to keep the patch small but it is better to add it now so that I don't forget.


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

https://reviews.llvm.org/D80181