[PATCH] D38645: [NVPTX] Implemented wmma intrinsics and instructions.

[Justin Lebar via Phabricator via llvm-commits]

jlebar added a comment.

> Take the address space optimization for an example, when we translate a generic load to specific load, we can just change the pointer type. The rests are just copied over.

Yeah, specializing on an inferred address space is an important optimization.  We were planning to handle this in tablegen, by pattern-matching `wmm.load/store(addrspacecast(ptr, x))`.  This should let it Just Work, without any need for an analysis beyond what we already have.



================
Comment at: llvm/include/llvm/IR/IntrinsicsNVVM.td:3882
+                      [regty, regty, regty, regty,
+                       regty, regty, regty, regty]),
+              !if(WithStride, [llvm_ptr_ty, llvm_i32_ty], [llvm_ptr_ty]),
----------------
I know it's tablegen and going to be ugly no matter what we do, but could we indent this as

```
!if(!eq(Abc#Type,"cf16"),
    [regty, regty, regty, regty],
    [regty, regty, regty, regty, regty, regty, regty, regty]),
```

?  Right now it looks like these arrays are parameters to `eq`, which is confusing.


================
Comment at: llvm/include/llvm/IR/IntrinsicsNVVM.td:3888
+                #!if(WithStride,".stride","")
+                #"."#Type>;
+
----------------
Any reason `Space` must contain a `.` but `Type` must not contain one?


================
Comment at: llvm/include/llvm/IR/IntrinsicsNVVM.td:3932
+                     regty, regty, regty, regty]),
+                !if(WithStride, [llvm_i32_ty], Empty)),
+              [], // Properties must be set during instantiation.
----------------
Should this be indented 2 fewer spaces?


================
Comment at: llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp:499
 
+enum WmmaVariant {
+  WMMA_VARIANT_ARI64,
----------------
This and the #define could use a brief comment, I think.


https://reviews.llvm.org/D38645