[Mlir-commits] [flang] [mlir] [flang][OpenACC] Fix implicit data mapping for deviceptr inside host_data (PR #192710)

[llvmlistbot at llvm.org]

https://github.com/khaki3 updated https://github.com/llvm/llvm-project/pull/192710

>From bff1f8dd730e411188b6208ee5b339f21823f144 Mon Sep 17 00:00:00 2001
From: Kazuaki Matsumura <kmatsumura at nvidia.com>
Date: Mon, 4 May 2026 10:20:40 -0700
Subject: [PATCH] [flang][OpenACC] Fix implicit data mapping for deviceptr
 inside host_data

When a subroutine with `!$acc data deviceptr(b)` containing `!$acc serial`
is inlined into a caller, the deviceptr's clause variable (embox of ref)
and the serial's use (ref) become different SSA values. The existing
alias analysis traces both back through the full def-chain via getSource(),
where an upstream sliced embox sets approximateSource=true, causing
alias() to conservatively return MayAlias instead of MustAlias. This
made ACCImplicitData generate copyin/copyout instead of deviceptr,
leading to a segfault at runtime.

Fix: add isOnZeroOffsetViewChain() to alias(Value, Value) that
short-circuits MustAlias when one value is directly derived from the
other through FortranObjectViewOpInterface ops with zero offset (e.g.
embox without slice, declare, box_addr). This avoids the full
getSource() walk where upstream approximate sources would conservatively
prevent MustAlias.

Co-authored-by: Cursor <cursoragent at cursor.com>
---
 .../lib/Optimizer/Analysis/AliasAnalysis.cpp  | 28 ++++++++++++
 .../Transforms/OpenACC/acc-implicit-data.fir  | 43 +++++++++++++++++++
 .../OpenACC/Transforms/ACCImplicitData.cpp    |  6 ++-
 3 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index 2509334b8ec4a..a075d720b7a71 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -365,10 +365,38 @@ static bool pathsDivergeAtComponent(const fir::AliasAnalysis::Source &lhsSrc,
   return false;
 }
 
+/// Walk backward from \p val through FortranObjectViewOpInterface ops
+/// that have zero offset (i.e. they access the same base address).
+/// Return true if \p other is found on this chain.
+static bool isOnZeroOffsetViewChain(mlir::Value val, mlir::Value other) {
+  while (auto *defOp = val.getDefiningOp()) {
+    auto viewOp = mlir::dyn_cast<fir::FortranObjectViewOpInterface>(defOp);
+    if (!viewOp)
+      break;
+    auto offset = viewOp.getViewOffset(mlir::cast<mlir::OpResult>(val));
+    if (!offset || *offset != 0)
+      break;
+    val = viewOp.getViewSource(mlir::cast<mlir::OpResult>(val));
+    if (val == other)
+      return true;
+  }
+  return false;
+}
+
 AliasResult AliasAnalysis::alias(mlir::Value lhs, mlir::Value rhs) {
   // A wrapper around alias(Source lhsSrc, Source rhsSrc, mlir::Value lhs,
   // mlir::Value rhs) This allows a user to provide Source that may be obtained
   // through other dialects
+
+  // If one value is directly derived from the other through a chain of
+  // zero-offset view operations (e.g. embox, declare, convert), they
+  // access the same underlying memory. This check avoids tracing through
+  // the full def-chain where upstream approximate sources (e.g. slices)
+  // would conservatively prevent MustAlias.
+  if (lhs == rhs || isOnZeroOffsetViewChain(lhs, rhs) ||
+      isOnZeroOffsetViewChain(rhs, lhs))
+    return AliasResult::MustAlias;
+
   auto lhsSrc = getSource(lhs);
   auto rhsSrc = getSource(rhs);
   return alias(lhsSrc, rhsSrc, lhs, rhs);
diff --git a/flang/test/Transforms/OpenACC/acc-implicit-data.fir b/flang/test/Transforms/OpenACC/acc-implicit-data.fir
index 050fe55747d23..e20e3231438d8 100644
--- a/flang/test/Transforms/OpenACC/acc-implicit-data.fir
+++ b/flang/test/Transforms/OpenACC/acc-implicit-data.fir
@@ -394,3 +394,46 @@ func.func private @_FortranAAllocatableSetBounds(!fir.ref<!fir.box<none>>, i32,
 // CHECK-NOT: acc.copyin
 // CHECK: acc.deviceptr
 // CHECK-NOT: acc.copyout
+
+// -----
+
+// Test that acc.serial inside acc.data deviceptr generates implicit deviceptr
+// (not copyin) when the deviceptr clause variable is derived from the ref used
+// by the serial construct (here, via fir.embox wrapping the declared ref).
+// This pattern arises when a subroutine containing:
+//   !$acc data deviceptr(b)    ← deviceptr operates on embox(b) (a box type)
+//     !$acc serial
+//       ... uses b ...         ← serial uses b directly (a ref type)
+//     !$acc end serial
+//   !$acc end data
+// is inlined into the caller. After inlining, the deviceptr's box and the
+// serial's ref are different SSA values with different types, so alias
+// analysis returns NoAlias. The pass must recognize that the deviceptr's
+// variable is derived from the ref and generate an implicit deviceptr clause
+// instead of falling back to copyin/copyout.
+func.func @test_serial_inside_data_deviceptr_embox() {
+  %c10 = arith.constant 10 : index
+  %c1 = arith.constant 1 : index
+  %arr = fir.alloca !fir.array<10xf32> {bindc_name = "b"}
+  %shape = fir.shape %c10 : (index) -> !fir.shape<1>
+  %arr_decl = fir.declare %arr(%shape) {uniq_name = "b"} : (!fir.ref<!fir.array<10xf32>>, !fir.shape<1>) -> !fir.ref<!fir.array<10xf32>>
+  %box = fir.embox %arr_decl(%shape) : (!fir.ref<!fir.array<10xf32>>, !fir.shape<1>) -> !fir.box<!fir.array<10xf32>>
+  %devptr = acc.deviceptr var(%box : !fir.box<!fir.array<10xf32>>) -> !fir.box<!fir.array<10xf32>> {name = "b(1:10)"}
+  acc.data dataOperands(%devptr : !fir.box<!fir.array<10xf32>>) {
+    acc.serial {
+      %elem = fir.array_coor %arr_decl(%shape) %c1 : (!fir.ref<!fir.array<10xf32>>, !fir.shape<1>, index) -> !fir.ref<f32>
+      %val = fir.load %elem : !fir.ref<f32>
+      acc.yield
+    }
+    acc.terminator
+  }
+  return
+}
+
+// CHECK-LABEL: func.func @test_serial_inside_data_deviceptr_embox
+// CHECK: acc.deviceptr var({{.*}} : !fir.box<!fir.array<10xf32>>) -> !fir.box<!fir.array<10xf32>> {name = "b(1:10)"}
+// CHECK: acc.data
+// CHECK: acc.deviceptr varPtr({{.*}} : !fir.ref<!fir.array<10xf32>>) -> !fir.ref<!fir.array<10xf32>> {implicit = true, name = "b"}
+// CHECK: acc.serial dataOperands({{.*}} : !fir.ref<!fir.array<10xf32>>)
+// CHECK-NOT: acc.copyin
+// CHECK-NOT: acc.copyout
diff --git a/mlir/lib/Dialect/OpenACC/Transforms/ACCImplicitData.cpp b/mlir/lib/Dialect/OpenACC/Transforms/ACCImplicitData.cpp
index 95c8d1076ccb0..0efce30c54677 100644
--- a/mlir/lib/Dialect/OpenACC/Transforms/ACCImplicitData.cpp
+++ b/mlir/lib/Dialect/OpenACC/Transforms/ACCImplicitData.cpp
@@ -311,9 +311,11 @@ Operation *ACCImplicitData::getOriginalDataClauseOpForAlias(
     if (auto *dataClauseOp = dataClause.getDefiningOp()) {
       // Only accept clauses that guarantee that the alias is present.
       if (isa<acc::CopyinOp, acc::CreateOp, acc::PresentOp, acc::NoCreateOp,
-              acc::DevicePtrOp>(dataClauseOp))
-        if (aliasAnalysis.alias(acc::getVar(dataClauseOp), var).isMust())
+              acc::DevicePtrOp>(dataClauseOp)) {
+        Value clauseVar = acc::getVar(dataClauseOp);
+        if (aliasAnalysis.alias(clauseVar, var).isMust())
           return dataClauseOp;
+      }
     }
   }
   return nullptr;