[Mlir-commits] [mlir] [mlir][openacc] Handle empty region in host fallback unwrap pattern (PR #205593)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-openacc

Author: Jiaqi He (heturing)

<details>
<summary>Changes</summary>

Fixes https://github.com/llvm/llvm-project/issues/205302.

The OpenACC host fallback unwrap pattern currently does not handle empty regions. It assumes that the region has exactly one block, but empty regions are accepted by existing OpenACC IR, such as `acc.serial {}`.

Although compiling from source does not seem to produce empty regions and instead generates regions such as `acc.serial { acc.yield }`, existing test cases use empty regions in many places. Also, `SerialOp::verify` does not reject empty regions. Therefore, this patch does not make this form of IR invalid.

Instead, this patch adds a special case for empty regions by simply erasing the wrapper operation. It also adds a regression test.

---
Full diff: https://github.com/llvm/llvm-project/pull/205593.diff


2 Files Affected:

- (modified) mlir/include/mlir/Dialect/OpenACC/Transforms/ACCSpecializePatterns.h (+4) 
- (modified) mlir/test/Dialect/OpenACC/acc-specialize-for-host-fallback.mlir (+10) 


``````````diff
diff --git a/mlir/include/mlir/Dialect/OpenACC/Transforms/ACCSpecializePatterns.h b/mlir/include/mlir/Dialect/OpenACC/Transforms/ACCSpecializePatterns.h
index cde85fe839bf5..8f0101f2ab79e 100644
--- a/mlir/include/mlir/Dialect/OpenACC/Transforms/ACCSpecializePatterns.h
+++ b/mlir/include/mlir/Dialect/OpenACC/Transforms/ACCSpecializePatterns.h
@@ -95,6 +95,10 @@ class ACCRegionUnwrapConversion : public OpRewritePattern<OpTy> {
 public:
   LogicalResult matchAndRewrite(OpTy op,
                                 PatternRewriter &rewriter) const override {
+    if (op.getRegion().empty()) {
+      rewriter.eraseOp(op);
+      return success();
+    }
     assert(op.getRegion().hasOneBlock() && "expected one block");
     Block *block = &op.getRegion().front();
     // Erase the terminator (acc.yield or acc.terminator) before unwrapping
diff --git a/mlir/test/Dialect/OpenACC/acc-specialize-for-host-fallback.mlir b/mlir/test/Dialect/OpenACC/acc-specialize-for-host-fallback.mlir
index 59269b71bf61c..2634dca305409 100644
--- a/mlir/test/Dialect/OpenACC/acc-specialize-for-host-fallback.mlir
+++ b/mlir/test/Dialect/OpenACC/acc-specialize-for-host-fallback.mlir
@@ -141,6 +141,16 @@ func.func @kernels_construct(%arg0 : memref<i32>) attributes {acc.routine_info =
   return
 }
 
+acc.routine @acc_routine_serial_empty func(@empty_serial_construct) seq
+// CHECK-LABEL: func.func @empty_serial_construct
+// CHECK-NOT:   acc.serial
+// CHECK:       return
+func.func @empty_serial_construct() attributes {acc.routine_info = #acc.routine_info<[@acc_routine_serial_empty]>} {
+  acc.serial {
+  }
+  return
+}
+
 //===----------------------------------------------------------------------===//
 // Declare enter/exit - erased (host fallback)
 //===----------------------------------------------------------------------===//

``````````

</details>


https://github.com/llvm/llvm-project/pull/205593