[PATCH] D79508: [mlir][gpu] Add utility functions to add private buffer attributions.

Stephan Herhut via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 7 05:04:10 PDT 2020


herhut requested changes to this revision.
herhut added a comment.
This revision now requires changes to proceed.

Thanks for adding this!



================
Comment at: mlir/include/mlir/Dialect/GPU/GPUOps.td:233
+    // private memory.
+    BlockArgument addPrivateAttribution(Type type) {
+      auto attrName = getNumPrivateAttributionsAttrName();
----------------
It is not clear to me why we have two versions of this function. While you have not caused this, could you clean it up and unify them? One could implement `addWorkgroupAttribution(ArrayRef<int64_t> shape, Type elementType)` by means of `BlockArgument addWorkgroupAttribution(Type type)`. 

I see only one use of the former form in `MemoryPromotion`. Changing that use to the type based one and removing the shape/elementType versions would be best.

Also, both of these could be implemented in the cpp file and only be declared here.


================
Comment at: mlir/include/mlir/Dialect/GPU/GPUOps.td:238
+
+      auto workgroupAttrName = getNumWorkgroupAttributionsAttrName();
+      auto workgroupAttr = getAttrOfType<IntegerAttr>(workgroupAttrName);
----------------
Use `getNumWorkgroupAttributions` here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79508





More information about the llvm-commits mailing list