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

[Stephan Herhut via Phabricator via llvm-commits]

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

Nice!



================
Comment at: mlir/test/Dialect/GPU/promotion.mlir:1
 // RUN: mlir-opt -allow-unregistered-dialect -test-gpu-memory-promotion -split-input-file %s | FileCheck %s
 
----------------
Maybe something like

```
// RUN: mlir-opt -allow-unregistered-dialect -pass-pipeline='module(gpu.module(gpu.func(test-gpu-memory-promotion)))' -split-input-file %s | FileCheck %s
```

is enough to direct the test pass to the functions through the extra wrapping modules.


================
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
----------------
Why this change?


================
Comment at: mlir/test/lib/Transforms/TestGpuMemoryPromotion.cpp:28
     : public PassWrapper<TestGpuMemoryPromotionPass,
-                         OperationPass<gpu::GPUFuncOp>> {
+                         OperationPass<gpu::GPUModuleOp>> {
   void runOnOperation() override {
----------------
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,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78541