[Mlir-commits] [mlir] Revert "[mlir][acc] Fold present() clauses on device values" (PR #215610)
Ivan R. Ivanov
llvmlistbot at llvm.org
Tue Aug 11 09:47:09 PDT 2026
https://github.com/ivanradanov created https://github.com/llvm/llvm-project/pull/215610
Reverts llvm/llvm-project#212815
Managed memory array may still be in the present table, but are classified as device memory in this pass, erroneously removing the present clause.
>From 19d6e63b71087296c52c06f06d8861a37e129708 Mon Sep 17 00:00:00 2001
From: "Ivan R. Ivanov" <iivanov at nvidia.com>
Date: Tue, 11 Aug 2026 18:45:12 +0200
Subject: [PATCH] Revert "[mlir][acc] Fold present() clauses on device values
(#212815)"
This reverts commit c405fd3870aadfce5a9cf1a13f0da8aaeb8d3854.
---
.../OpenACC/Transforms/ACCImplicitData.cpp | 76 +++++--------------
.../Dialect/OpenACC/acc-implicit-data.mlir | 46 -----------
2 files changed, 20 insertions(+), 102 deletions(-)
diff --git a/mlir/lib/Dialect/OpenACC/Transforms/ACCImplicitData.cpp b/mlir/lib/Dialect/OpenACC/Transforms/ACCImplicitData.cpp
index 5aae7dfef7d58..628454905b488 100644
--- a/mlir/lib/Dialect/OpenACC/Transforms/ACCImplicitData.cpp
+++ b/mlir/lib/Dialect/OpenACC/Transforms/ACCImplicitData.cpp
@@ -215,7 +215,6 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/TypeSwitch.h"
-#include "llvm/Support/Casting.h"
#include "llvm/Support/ErrorHandling.h"
#include <type_traits>
@@ -458,6 +457,15 @@ Operation *ACCImplicitData::generateDataClauseOpForCandidate(
typeCategory, acc::VariableTypeCategory::aggregate);
Location loc = computeConstructOp->getLoc();
+ if (acc::isDeviceValue(var)) {
+ // If the variable is device data, use deviceptr clause.
+ LLVM_DEBUG(llvm::dbgs() << "Using deviceptr clause because variable is "
+ "device data\n");
+ return acc::DevicePtrOp::create(builder, loc, var,
+ /*structured=*/true, /*implicit=*/true,
+ accSupport.getVariableName(var));
+ }
+
Operation *op = nullptr;
op = getOriginalDataClauseOpForAlias(var, builder, computeConstructOp,
dominatingDataClauses);
@@ -482,16 +490,6 @@ Operation *ACCImplicitData::generateDataClauseOpForCandidate(
acc::getBounds(op));
}
- if (acc::isDeviceValue(var)) {
- // Variable is device data with no existing dominating mapping: use
- // deviceptr clause.
- LLVM_DEBUG(llvm::dbgs() << "Using deviceptr clause because variable is "
- "device data\n");
- return acc::DevicePtrOp::create(builder, loc, var,
- /*structured=*/true, /*implicit=*/true,
- accSupport.getVariableName(var));
- }
-
if (isScalar) {
if (enableImplicitReductionCopy &&
acc::isOnlyUsedByReductionClauses(var,
@@ -704,35 +702,6 @@ static void insertInSortedOrder(SmallVector<Value> &sortedDataClauseOperands,
}
}
-/// A present() clause on a device value always holds. Erase it to allow the
-/// implicit data to generate an acc.deviceptr for it.
-template <typename OpT>
-static void foldPresentDeviceValue(OpT computeConstructOp) {
- SmallVector<Value> remainingOperands;
- SmallVector<acc::PresentOp> toErase;
- for (Value var : computeConstructOp.getDataClauseOperands()) {
- if (auto presentOp =
- dyn_cast_if_present<acc::PresentOp>(var.getDefiningOp())) {
- if (acc::isDeviceValue(presentOp.getVar())) {
- toErase.push_back(presentOp);
- continue;
- }
- }
- remainingOperands.push_back(var);
- }
- if (toErase.empty())
- return;
-
- computeConstructOp.getDataClauseOperandsMutable().assign(remainingOperands);
- for (acc::PresentOp presentOp : toErase) {
- Operation *exitOp = findDataExitOp(presentOp);
- assert(exitOp && exitOp->getNumResults() == 0);
- presentOp.getAccVar().replaceAllUsesWith(presentOp.getVar());
- exitOp->erase();
- presentOp->erase();
- }
-}
-
template <typename OpT>
void ACCImplicitData::generateImplicitDataOps(
ModuleOp &module, OpT computeConstructOp,
@@ -821,24 +790,19 @@ void ACCImplicitData::runOnOperation() {
acc::OpenACCSupport &accSupport = getAnalysis<acc::OpenACCSupport>();
- SmallVector<Operation *> computeConstructOps;
module.walk([&](Operation *op) {
- if (isa<ACC_COMPUTE_CONSTRUCT_OPS, acc::KernelEnvironmentOp>(op))
- computeConstructOps.push_back(op);
+ if (isa<ACC_COMPUTE_CONSTRUCT_OPS, acc::KernelEnvironmentOp>(op)) {
+ assert(op->getNumRegions() == 1 && "must have 1 region");
+
+ auto defaultClause = acc::getDefaultAttr(op);
+ llvm::TypeSwitch<Operation *, void>(op)
+ .Case<ACC_COMPUTE_CONSTRUCT_OPS, acc::KernelEnvironmentOp>(
+ [&](auto op) {
+ generateImplicitDataOps(module, op, defaultClause, accSupport);
+ })
+ .Default([&](Operation *) {});
+ }
});
-
- for (Operation *op : computeConstructOps) {
- assert(op->getNumRegions() == 1 && "must have 1 region");
-
- auto defaultClause = acc::getDefaultAttr(op);
- llvm::TypeSwitch<Operation *, void>(op)
- .Case<ACC_COMPUTE_CONSTRUCT_OPS, acc::KernelEnvironmentOp>(
- [&](auto op) {
- foldPresentDeviceValue(op);
- generateImplicitDataOps(module, op, defaultClause, accSupport);
- })
- .Default([&](Operation *) {});
- }
}
} // namespace
diff --git a/mlir/test/Dialect/OpenACC/acc-implicit-data.mlir b/mlir/test/Dialect/OpenACC/acc-implicit-data.mlir
index 6551b20100005..3b6b5e1ade5e0 100644
--- a/mlir/test/Dialect/OpenACC/acc-implicit-data.mlir
+++ b/mlir/test/Dialect/OpenACC/acc-implicit-data.mlir
@@ -287,49 +287,3 @@ func.func @test_declare_deviceptr_arg_in_parallel(%arg0: memref<?xi8>) {
// CHECK: acc.declare_exit token(%[[TOKEN]]) dataOperands(%[[DEVPTR]] : memref<10xf32>)
// CHECK-NOT: acc.copyin
// CHECK-NOT: acc.copyout
-
-// -----
-
-// Fold an explicit present of device data: drop present/delete and rewrite
-// region uses; subsequent implicit mapping should emit deviceptr.
-func.func @test_fold_present_device_value() {
- %alloc = memref.alloca() : memref<10xf32, #gpu.address_space<global>>
- %present = acc.present varPtr(%alloc : memref<10xf32, #gpu.address_space<global>>) -> memref<10xf32, #gpu.address_space<global>> {name = "a"}
- acc.parallel dataOperands(%present : memref<10xf32, #gpu.address_space<global>>) {
- %c0 = arith.constant 0 : index
- %load = memref.load %present[%c0] : memref<10xf32, #gpu.address_space<global>>
- acc.yield
- }
- acc.delete accPtr(%present : memref<10xf32, #gpu.address_space<global>>) {dataClause = #acc<data_clause acc_present>, name = "a"}
- return
-}
-
-// CHECK-LABEL: func.func @test_fold_present_device_value
-// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref<10xf32, #gpu.address_space<global>>
-// CHECK: %[[DEVPTR:.*]] = acc.deviceptr varPtr(%[[ALLOC]] : memref<10xf32, #gpu.address_space<global>>) -> memref<10xf32, #gpu.address_space<global>> {implicit = true, name = ""}
-// CHECK: acc.parallel dataOperands(%[[DEVPTR]] : memref<10xf32, #gpu.address_space<global>>) {
-// CHECK: memref.load %[[DEVPTR]][{{.*}}] : memref<10xf32, #gpu.address_space<global>>
-// CHECK-NOT: acc.present
-// CHECK-NOT: acc.delete
-
-// -----
-
-// Present of host data must not be folded away.
-func.func @test_present_host_not_folded() {
- %alloc = memref.alloca() : memref<10xf32>
- %present = acc.present varPtr(%alloc : memref<10xf32>) -> memref<10xf32> {name = "a"}
- acc.parallel dataOperands(%present : memref<10xf32>) {
- %c0 = arith.constant 0 : index
- %load = memref.load %present[%c0] : memref<10xf32>
- acc.yield
- }
- acc.delete accPtr(%present : memref<10xf32>) {dataClause = #acc<data_clause acc_present>, name = "a"}
- return
-}
-
-// CHECK-LABEL: func.func @test_present_host_not_folded
-// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref<10xf32>
-// CHECK: %[[PRESENT:.*]] = acc.present varPtr(%[[ALLOC]] : memref<10xf32>) -> memref<10xf32> {name = "a"}
-// CHECK: acc.parallel dataOperands(%[[PRESENT]] : memref<10xf32>) {
-// CHECK: memref.load %[[PRESENT]][{{.*}}] : memref<10xf32>
-// CHECK: acc.delete accPtr(%[[PRESENT]] : memref<10xf32>) {dataClause = #acc<data_clause acc_present>, name = "a"}
More information about the Mlir-commits
mailing list