Declare effects on air.dma_memcpy_nd; migrate FoldExecute to mightHaveEffect (#1575)

* Declare memory effects on air.dma_memcpy_nd

Read on src, Write on dst — mirrors memref.copy's MemReadAt/MemWriteAt
declarations. Without this the op was opaque to side-effect queries:
ExecuteOp::getEffects had to fall back to conservative R+W+Free for
any execute body that contained a DMA (the catch-all sawUnknownOp
branch); FoldExecute's hand-rolled Write check silently skipped DMA
ops; LocalAliasAnalysis::getModRef returned MayMod/MayRef for them.

air.channel.put / air.channel.get are intentionally left out —
modelling them correctly requires a side effect on the channel symbol
(not just the local memref), since put has no Write to memory but is
still observable. That needs a custom Resource and is filed as a
followup.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Migrate FoldExecute side-effect check to upstream helper

Replace the hand-rolled walk over body.without_terminator() with
mlir::mightHaveEffect<MemoryEffects::Write>(op). ExecuteOp::getEffects
already mirrors body effects up; the upstream helper additionally
returns true when the body contains an unknown-effect op, which the
old walk silently skipped (latent unsafety: any effect-bearing op
without MemoryEffectOpInterface declared was treated as no-Write).

execute_3's expectation changes accordingly: its body contains a
dma_memcpy_nd that now reports a Write on dst, so the execute is
conservatively kept. Folding it is only safe if the dst is provably
body-local; that needs escape analysis and is left as a followup.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Use upstream hasEffect in checkOpOperandReadOrWrite

Replace the explicit air.dma_memcpy_nd branch with a generic
mlir::hasEffect<MemoryEffects::{Read,Write}> query. Now that the DMA
op declares MemReadAt<src>/MemWriteAt<dst>, the upstream helper
returns the same answer the dialect-specific switch did, and any
future op that declares effects is picked up automatically.

The air.channel.put/get and linalg branches stay because channels
do not yet declare effects (separate followup) and the linalg path
uses the DPS interface, not memory effects.

Two callers (AIRDependencyScheduleOpt.cpp:1126/1146,
Dependency.cpp:2928) benefit transparently — the return-code contract
is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: erwei-xilinx

mlir/include/air/Dialect/AIR/AIR.td

@@ -466,11 +466,13 @@ def air_DmaMemcpyNdOp: air_Op<"dma_memcpy_nd",
   let summary = "dma operator";
   let arguments = (
     ins Variadic<air_AsyncToken>:$async_dependencies,
-        AnyRankedOrUnrankedMemRef:$dst,
+        Arg<AnyRankedOrUnrankedMemRef, "destination memref",
+            [MemWriteAt<0, FullEffect>]>:$dst,
         Variadic<Index>:$dst_offsets,
         Variadic<Index>:$dst_sizes,
         Variadic<Index>:$dst_strides,
-        AnyRankedOrUnrankedMemRef:$src,
+        Arg<AnyRankedOrUnrankedMemRef, "source memref",
+            [MemReadAt<0, FullEffect>]>:$src,
         Variadic<Index>:$src_offsets,
         Variadic<Index>:$src_sizes,
         Variadic<Index>:$src_strides,

mlir/lib/Dialect/AIR/IR/AIRDialect.cpp

@@ -2103,19 +2103,10 @@ static LogicalResult FoldExecute(air::ExecuteOp op, PatternRewriter &rewriter) {
 
   // if we get here then only the async token result has uses.
 
-  // Don't fold if body contains ops with memory write side effects
-  // (e.g., memref.store). These ops must be preserved even if the
-  // execute's async token becomes unused due to wait_all folding.
-  for (auto &bodyOp : body.without_terminator()) {
-    if (auto memEffects =
-            dyn_cast_if_present<MemoryEffectOpInterface>(bodyOp)) {
-      SmallVector<MemoryEffects::EffectInstance> effects;
-      memEffects.getEffects(effects);
-      for (auto &effect : effects)
-        if (isa<MemoryEffects::Write>(effect.getEffect()))
-          return failure();
-    }
-  }
+  // ExecuteOp::getEffects mirrors body effects up; an inner Write or any
+  // unknown-effect op blocks folding.
+  if (mlir::mightHaveEffect<MemoryEffects::Write>(op))
+    return failure();
 
   // if there are extra results than async token, and none of them are used,
   // then replace the execute with a wait_all no-op.

mlir/lib/Util/Util.cpp

@@ -824,19 +824,18 @@ char air::checkOpOperandReadOrWrite(Value v, Operation *owner) {
 }
 char air::checkOpOperandReadOrWrite(mlir::OpOperand &op_operand) {
   auto owner = op_operand.getOwner();
-  // If used in DmaMemcpy Op
-  if (auto dma = dyn_cast_if_present<xilinx::air::DmaMemcpyNdOp>(owner)) {
-    if (op_operand.is(dma.getSrcMemref())) {
-      return 'r';
-    } else if (op_operand.is(dma.getDstMemref())) {
-      return 'w';
-    } else {
-      return 'u';
-    }
-  }
+  // Generic side-effect query first. air.dma_memcpy_nd declares
+  // MemReadAt<src> + MemWriteAt<dst>, so it answers here without an
+  // explicit dialect branch. air.channel.put/get do not yet declare
+  // effects (channel-resource modeling is a separate followup) and
+  // continue to use the dialect-specific branches below.
+  if (mlir::hasEffect<mlir::MemoryEffects::Write>(owner, op_operand.get()))
+    return 'w';
+  if (mlir::hasEffect<mlir::MemoryEffects::Read>(owner, op_operand.get()))
+    return 'r';
   // If used in Channel Put Op
-  else if (auto channel_put =
-               dyn_cast_if_present<xilinx::air::ChannelPutOp>(owner)) {
+  if (auto channel_put =
+          dyn_cast_if_present<xilinx::air::ChannelPutOp>(owner)) {
     if (op_operand.is(channel_put.getSrc())) {
       return 'r';
     } else {

mlir/test/Dialect/AIR/air_canonicalize.mlir

@@ -291,10 +291,15 @@ func.func @execute_2() -> (index, !air.async.token) {
   return %results, %t : index, !air.async.token
 }
 
+// air.dma_memcpy_nd has a Write effect on its dst, so the execute body
+// is conservatively kept by FoldExecute. The unused %results index is
+// still replaced by the constant.
 // CHECK-LABEL: execute_3
 // CHECK-NEXT: arith.constant 0 : index
-// CHECK-NEXT: air.wait_all async 
-// CHECK-NEXT: return %{{.*}}, %{{.*}} : index, !air.async.token
+// CHECK-NEXT: air.execute
+// CHECK-NEXT: memref.alloc
+// CHECK-NEXT: air.dma_memcpy_nd
+// CHECK: return %{{.*}}, %{{.*}} : index, !air.async.token
 func.func @execute_3(%arg0: memref<1xi32>) -> (index, !air.async.token) {
   %c0 = arith.constant 0 : index
   %async_token, %results = air.execute -> (index) {