[PATCH] D78541: [MLIR] Ensure `gpu.func` must be inside a `gpu.module`.

Frederik Gossen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 21 03:12:46 PDT 2020


frgossen added inline comments.


================
Comment at: mlir/test/Dialect/GPU/promotion.mlir:11
+    gpu.func @memref3d(%arg0: memref<5x4xf32> {gpu.test_promote_workgroup}) kernel {
+      // verify that loop bounds are emitted, the order does not matter.
+      // CHECK-DAG: %[[c1:.*]] = constant 1
----------------
herhut wrote:
> Why this change?
Was not intended. Locally, I did not see this because all the indented lines appear as a change.


================
Comment at: mlir/test/Dialect/GPU/promotion.mlir:55
 
-module @foo attributes {gpu.kernel_module} {
-  // Verify that the attribution was indeed introduced
-  // CHECK-LABEL: @memref5d
-  // CHECK-SAME: (%[[arg:.*]]: memref<8x7x6x5x4xf32>
-  // CHECK-SAME: workgroup(%[[promoted:.*]] : memref<8x7x6x5x4xf32, 3>)
-  gpu.func @memref5d(%arg0: memref<8x7x6x5x4xf32> {gpu.test_promote_workgroup}) kernel {
-    // Verify that loop bounds are emitted, the order does not matter.
-    // CHECK-DAG: %[[c0:.*]] = constant 0
-    // CHECK-DAG: %[[c1:.*]] = constant 1
-    // CHECK-DAG: %[[c4:.*]] = constant 4
-    // CHECK-DAG: %[[c5:.*]] = constant 5
-    // CHECK-DAG: %[[c6:.*]] = constant 6
-    // CHECK-DAG: %[[c7:.*]] = constant 7
-    // CHECK-DAG: %[[c8:.*]] = constant 8
-    // CHECK-DAG: %[[tx:.*]] = "gpu.thread_id"() {dimension = "x"}
-    // CHECK-DAG: %[[ty:.*]] = "gpu.thread_id"() {dimension = "y"}
-    // CHECK-DAG: %[[tz:.*]] = "gpu.thread_id"() {dimension = "z"}
-    // CHECK-DAG: %[[bdx:.*]] = "gpu.block_dim"() {dimension = "x"}
-    // CHECK-DAG: %[[bdy:.*]] = "gpu.block_dim"() {dimension = "y"}
-    // CHECK-DAG: %[[bdz:.*]] = "gpu.block_dim"() {dimension = "z"}
-
-    // Verify that loops for the copy are emitted.
-    // CHECK: loop.for %[[i0:.*]] =
-    // CHECK:   loop.for %[[i1:.*]] =
-    // CHECK:     loop.for %[[i2:.*]] =
-    // CHECK:       loop.for %[[i3:.*]] =
-    // CHECK:         loop.for %[[i4:.*]] =
-
-    // Verify that the copy is emitted.
-    // CHECK:           %[[v:.*]] = load %[[arg]][%[[i0]], %[[i1]], %[[i2]], %[[i3]], %[[i4]]]
-    // CHECK:           store %[[v]], %[[promoted]][%[[i0]], %[[i1]], %[[i2]], %[[i3]], %[[i4]]]
-
-    // Verify that the use has been rewritten.
-    // CHECK: "use"(%[[promoted]]) : (memref<8x7x6x5x4xf32, 3>)
-    "use"(%arg0) : (memref<8x7x6x5x4xf32>) -> ()
-
-    // Verify that loop loops for the copy are emitted.
-    // CHECK: loop.for %[[i0:.*]] =
-    // CHECK:   loop.for %[[i1:.*]] =
-    // CHECK:     loop.for %[[i2:.*]] =
-    // CHECK:       loop.for %[[i3:.*]] =
-    // CHECK:         loop.for %[[i4:.*]] =
-
-    // Verify that the copy is emitted.
-    // CHECK:           %[[v:.*]] = load %[[promoted]][%[[i0]], %[[i1]], %[[i2]], %[[i3]], %[[i4]]]
-    // CHECK:           store %[[v]], %[[arg]][%[[i0]], %[[i1]], %[[i2]], %[[i3]], %[[i4]]]
-    gpu.return
+module {
+  gpu.module @foo {
----------------
rriddle wrote:
> You can remove all of the empty module wrappers, module is implicitly the top-level operation.
Didn't know that. Thanks! 


================
Comment at: mlir/test/lib/Transforms/TestGpuMemoryPromotion.cpp:28
     : public PassWrapper<TestGpuMemoryPromotionPass,
-                         OperationPass<gpu::GPUFuncOp>> {
+                         OperationPass<gpu::GPUModuleOp>> {
   void runOnOperation() override {
----------------
herhut wrote:
> rriddle wrote:
> > Can you just change the pipeline specification in the tests instead of changing this pass?
> See my above comment for how to do this. You can then revert these changes,
That's much nicer. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78541





More information about the llvm-commits mailing list