Skip to content

Conversation

@VijayKandiah
Copy link
Contributor

This change removes empty acc.kernel_environment operations during canonicalization. This could happen when the acc compute construct inside the acc.kernel_environment is optimized away in cases such as when only private variables are being written to in the loop.

In cases of empty acc.kernel_environment ops with waitOperands, we still remove the empty acc.kernel_environment, but also create an acc.wait operation to take those wait operands to preserve synchronization behavior.

@llvmbot
Copy link
Member

llvmbot commented Nov 5, 2025

@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-openacc

@llvm/pr-subscribers-mlir-openacc

Author: Vijay Kandiah (VijayKandiah)

Changes

This change removes empty acc.kernel_environment operations during canonicalization. This could happen when the acc compute construct inside the acc.kernel_environment is optimized away in cases such as when only private variables are being written to in the loop.

In cases of empty acc.kernel_environment ops with waitOperands, we still remove the empty acc.kernel_environment, but also create an acc.wait operation to take those wait operands to preserve synchronization behavior.


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

3 Files Affected:

  • (modified) mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td (+2)
  • (modified) mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp (+40)
  • (modified) mlir/test/Dialect/OpenACC/canonicalize.mlir (+26)
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index c689b7e46ea9e..5b89f741e296d 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -2184,6 +2184,8 @@ def OpenACC_KernelEnvironmentOp : OpenACC_Op<"kernel_environment",
     )
     $region attr-dict
   }];
+
+  let hasCanonicalizer = 1;
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index b2f1d840f3bca..fa303ed675daf 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -1042,6 +1042,37 @@ struct RemoveConstantIfConditionWithRegion : public OpRewritePattern<OpTy> {
   }
 };
 
+/// Remove empty acc.kernel_environment operations. If the operation has wait
+/// operands, create a acc.wait operation to preserve synchronization.
+struct RemoveEmptyKernelEnvironment
+    : public OpRewritePattern<acc::KernelEnvironmentOp> {
+  using OpRewritePattern<acc::KernelEnvironmentOp>::OpRewritePattern;
+
+  LogicalResult matchAndRewrite(acc::KernelEnvironmentOp op,
+                                PatternRewriter &rewriter) const override {
+    assert(op->getNumRegions() == 1 && "expected op to have one region");
+
+    Block &block = op.getRegion().front();
+    if (!block.empty())
+      return failure();
+
+    // Remove empty kernel environment
+    // preserve synchronization by creating acc.wait operation if needed
+    if (!op.getWaitOperands().empty()) {
+      rewriter.replaceOpWithNewOp<acc::WaitOp>(
+          op,
+          /*waitOperands=*/op.getWaitOperands(),
+          /*asyncOperand=*/Value(),
+          /*waitDevnum=*/Value(),
+          /*async=*/nullptr,
+          /*ifCond=*/Value());
+    } else
+      rewriter.eraseOp(op);
+
+    return success();
+  }
+};
+
 //===----------------------------------------------------------------------===//
 // Recipe Region Helpers
 //===----------------------------------------------------------------------===//
@@ -2690,6 +2721,15 @@ void acc::HostDataOp::getCanonicalizationPatterns(RewritePatternSet &results,
   results.add<RemoveConstantIfConditionWithRegion<HostDataOp>>(context);
 }
 
+//===----------------------------------------------------------------------===//
+// KernelEnvironmentOp
+//===----------------------------------------------------------------------===//
+
+void acc::KernelEnvironmentOp::getCanonicalizationPatterns(
+    RewritePatternSet &results, MLIRContext *context) {
+  results.add<RemoveEmptyKernelEnvironment>(context);
+}
+
 //===----------------------------------------------------------------------===//
 // LoopOp
 //===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/OpenACC/canonicalize.mlir b/mlir/test/Dialect/OpenACC/canonicalize.mlir
index fdc8e6b5cae6e..6d600a386f926 100644
--- a/mlir/test/Dialect/OpenACC/canonicalize.mlir
+++ b/mlir/test/Dialect/OpenACC/canonicalize.mlir
@@ -219,3 +219,29 @@ func.func @update_unnecessary_computations(%x: memref<i32>) {
 // CHECK-LABEL: func.func @update_unnecessary_computations
 // CHECK-NOT: acc.atomic.update
 // CHECK: acc.atomic.write
+
+// -----
+
+func.func @remove_empty_kernel_environment() {
+  acc.kernel_environment {
+  }
+  return
+}
+
+// CHECK-LABEL: func.func @remove_empty_kernel_environment
+// CHECK-NOT: acc.kernel_environment
+// CHECK: return
+
+// -----
+
+func.func @kernel_environment_with_wait(%q1: i32, %q2: i32) {
+  acc.kernel_environment wait({%q1 : i32, %q2 : i32}) {
+  }
+  return
+}
+
+// CHECK-LABEL: func.func @kernel_environment_with_wait
+// CHECK-SAME: ([[Q1:%.*]]: i32, [[Q2:%.*]]: i32)
+// CHECK-NOT: acc.kernel_environment
+// CHECK: acc.wait([[Q1]], [[Q2]] : i32, i32)
+// CHECK: return

@github-actions
Copy link

github-actions bot commented Nov 5, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@razvanlupusoru razvanlupusoru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@VijayKandiah VijayKandiah merged commit faae161 into llvm:main Nov 6, 2025
10 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 6, 2025

LLVM Buildbot has detected a new failure on builder ppc64le-mlir-rhel-clang running on ppc64le-mlir-rhel-test while building mlir at step 6 "test-build-check-mlir-build-only-check-mlir".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/129/builds/32615

Here is the relevant piece of the build log for the reference
Step 6 (test-build-check-mlir-build-only-check-mlir) failure: 1200 seconds without output running [b'ninja', b'check-mlir'], attempting to kill
...
PASS: MLIR :: mlir-tblgen/op-error.td (3629 of 3640)
PASS: MLIR-Unit :: IR/./MLIRIRTests/0/130 (3630 of 3640)
PASS: MLIR-Unit :: IR/./MLIRIRTests/101/130 (3631 of 3640)
PASS: MLIR-Unit :: Interfaces/./MLIRInterfacesTests/11/22 (3632 of 3640)
PASS: MLIR-Unit :: IR/./MLIRIRTests/38/130 (3633 of 3640)
PASS: MLIR-Unit :: Interfaces/./MLIRInterfacesTests/13/22 (3634 of 3640)
PASS: MLIR-Unit :: Pass/./MLIRPassTests/10/13 (3635 of 3640)
PASS: MLIR-Unit :: Interfaces/./MLIRInterfacesTests/12/22 (3636 of 3640)
PASS: MLIR-Unit :: IR/./MLIRIRTests/39/130 (3637 of 3640)
PASS: MLIR :: mlir-reduce/dce-test.mlir (3638 of 3640)
command timed out: 1200 seconds without output running [b'ninja', b'check-mlir'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=3130.151393

vinay-deshmukh pushed a commit to vinay-deshmukh/llvm-project that referenced this pull request Nov 8, 2025
llvm#166633)

This change removes empty `acc.kernel_environment` operations during
canonicalization. This could happen when the acc compute construct
inside the `acc.kernel_environment` is optimized away in cases such as
when only private variables are being written to in the loop.

In cases of empty `acc.kernel_environment` ops with waitOperands, we
still remove the empty `acc.kernel_environment`, but also create an
`acc.wait` operation to take those wait operands to preserve
synchronization behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants