fix[next-dace]: detect write-after-read hazard in copy chain removal

[edopao]

Description

CopyChainRemover merges a (A1) -> (A2) copy by removing one container and
rewiring the flow onto the survivor. Depending on the mode this moves either the
writes or the reads of the removed transient:

• PULL mode ((T) -> (G), survivor G): the transient T is removed and
  its writes are taken over by G, which moves them back to where T was
  produced.
• PUSH mode ((G) -> (T), survivor G): the transient T is removed and
  its consumers are rerouted to read from G directly, which moves the read of
  G downstream to where T was consumed.

When the surviving container is a global that is also accessed elsewhere in
the same state, the removed transient was acting as a double buffer and the move
can let a write overtake a read (PULL) or a read overtake a write (PUSH),
yielding silently wrong results. This is the pattern generated by an in-place
update stencil, i.e. a field that a field_operator call takes as an input and
also produces as an output.

Fix

Add _creates_write_after_read_hazard and split it by copy-chain mode:

• PULL: the write to G is moved upstream, so a read of G upstream of
  the survivor can be overtaken. Scan the ancestor region of the survivor for
  reads of the same data.
• PUSH: the read of G is moved downstream, so a write to G between the
  survivor and where the transient was consumed can be overtaken. Scan the
  descendant region of the survivor for writes to the same data.

Transients are written only once (ADR-18), so they are unaffected and are
skipped by the check. As part of the PUSH scan, find_downstream_nodes is added
as the symmetric counterpart to find_upstream_nodes.

Tests

• test_copy_chain_destination_read_in_map_no_apply reproduces the in-place
  update pattern in PULL mode:

  for i in range(10):
      d[i] = c[i] + 1.0  # `c` read in a branch independent of the producer of `b`.
      b[i] = a[i] * 2.0
  c[:] = b[:]  # Copy chain `(b) -> (c)`: the surviving global `c` is read in the Map.

  The (b) -> (c) copy chain must not be removed, since c survives and is
  still read in the Map.

• test_push_chain_source_written_in_map_no_apply mirrors the above for
  PUSH mode:

  b[:] = a[:]  # Copy chain `(a) -> (b)`: the surviving global `a` is copied into `b`.
  for i in range(10):
      d[i] = b[i] + 1.0  # `b` read in a branch (old value of `a`).
      a[i] = a[i] * 2.0  # `a` written again in another branch.

  The (a) -> (b) copy chain must not be removed, since a survives and is
  written again in the Map before the consumer reads.

Both tests compile and run the SDFG before and after, assert the transformation
does not apply, and that the transient double buffer is retained. Each was
verified to fail when the corresponding guard is neutralized, confirming the
tests reproduce the issue.

Notes

This is one of a family of write-after-read fixes for in-place update stencils on
the DaCe backend (cf. #2815 GT4PyMapBufferElimination, #2816 DoubleWriteRemover,
#2817 GT4PyStateFusion). It targets a distinct transformation (CopyChainRemover)
and does not overlap with those PRs; the PULL reproducer deliberately mirrors the SDFG
shape used in #2816. The fix was extracted from the dace_refactor_lowering draft
(#2690) into this standalone PR.

[philip-paul-mueller]

I agree that this is a similar but different kind of bug than the other ones and it is a real defect in the transformation.
The changes looks okay, however, I think that we do not need to scan all nodes in the graph, but it is enough to perform the test on the upstream nodes (although it could even be reduced more, by recognizing that a global access node acts as a serialization point.

[philip-paul-mueller]

It looks good to me, but there is one question that I have and might trigger another round.
But if the answer is negative, i.e. this case is harmless, then we can approve it right away.

[philip-paul-mueller]

LGTM some modified comments.
It is important that we do a performance test to see if there are changes.

[philip-paul-mueller]

Please keep this note but reformulate it to:
NOTE: ADR18 technically only allows for point-wise dependencies for global accesses, however, to support ICON4Py this is sometimes not fully validated, see test_copy_chain_destination_read_in_map_no_apply() for an example.
The issue could be solved by adding empty Memlets, but the pipeline support of them outside of Maps is not very mature.
The simplest thing would be to forbid inout variables altogether.
Thus we simply forbid this.

[edopao]

I have updated the comment., but differently.

[edopao]

After analysis, it is very difficult to detect all valid cases, which do not represent a write-after-read hazard.