[flang-commits] [flang] [llvm] [flang-rt] Copy back only modified elements in CopyOutAssign (PR #222101)

Eugene Epshteyn via flang-commits flang-commits at lists.llvm.org
Tue Sep 8 11:57:11 PDT 2026


https://github.com/eugeneepshteyn created https://github.com/llvm/llvm-project/pull/222101

**What**

`CopyOutAssign()` used to copy the whole temporary back into the original variable unconditionally. Since `CopyInAssign()` creates the temporary as a bitwise copy of the variable, an element the callee never modified is still bitwise-identical to the original, so storing it back is unnecessary. This PR makes copy-out compare each element bitwise and store only the elements that differ.

**Why**

A compiler-generated copy-out can write into storage that must not be written even in a conforming program. Example (from the discussion in #187348): an `INTENT(IN)` dummy backed by read-only storage is forwarded to a procedure without declared intent, which passes a non-contiguous section onward — the resulting copy-in/copy-out writes unmodified bytes back into read-only memory and crashes, although nothing was ever modified. Making copy-out store-free for unmodified data removes this failure mode, and is a prerequisite for passing non-definable actual arguments (such as named constants) without defensive temporaries (see #187348 and #197636).

The comparison is bitwise (`memcmp`), not a value comparison, so unmodified NaN elements and derived-type padding do not produce spurious stores: the temporary is produced by `ShallowCopy()` (byte copy including padding), so untouched elements are bit-identical. An element the callee did assign may differ only in padding; storing it back is correct since it was genuinely modified.

**Escape hatch**

The environment variable `FLANG_RT_COPYOUT_MODIFIED_ONLY` (documented in `flang/docs/RuntimeEnvironment.md`) defaults to `1`; setting it to `0` restores the previous unconditional copy-out, for triage and comparison.

**Implementation notes**

The new `ShallowCopyModifiedElements()` mirrors the existing `ShallowCopy*` type/rank specialization structure so the per-element comparison and copy inline to fixed-size operations; an earlier generic (per-element `memcmp` libcall) version measured 2-3x slower than the unconditional copy, while the specialized version measures at parity within measurement noise. One behavioral note: for a `VOLATILE` original involved in copy-in/copy-out (already hazardous per F2023 15.5.2.4 NOTE 5), copy-out now performs compare reads and skips equal-value stores.

**Testing**

- `check-flang-rt` passes; three new unit tests in `flang-rt/unittests/Runtime/Assign.cpp`: a strided round-trip (modified elements land, untouched elements intact), a POSIX-guarded proof that an unmodified copy-out performs no stores at all (original placed on an `mprotect(PROT_READ)` page, including a NaN element), and a death test showing `FLANG_RT_COPYOUT_MODIFIED_ONLY=0` restores the storing behavior.
- Performance measurements on x86_64 (up to 512 MB temporaries, read-only and write-all callees) show parity within noise; I will attach cleaner numbers, including aarch64 runs, to this PR.

**Draft status**

Posting as draft while performance measurements on additional configurations are collected; follow-up changes will be added as new commits to this PR.


>From 5abbfcd8f41dc264f5497dd4afb7b20f1f0e8626 Mon Sep 17 00:00:00 2001
From: Eugene Epshteyn <eepshteyn at nvidia.com>
Date: Tue, 8 Sep 2026 11:22:12 -0700
Subject: [PATCH] [flang-rt] Copy back only modified elements in CopyOutAssign

CopyOutAssign() used to copy the whole temporary back into the original
variable unconditionally. Since CopyInAssign() creates the temporary as a
bitwise copy of the variable, an element the callee never modified is
still bitwise-identical to the original, and storing it back is
unnecessary.

Compare each element bitwise and store only the elements that differ.
This avoids compiler-generated stores into storage that must not be
written -- e.g. an actual argument that is not definable and was,
conformingly, never modified by the callee -- which is a prerequisite for
passing non-definable actual arguments (such as named constants) without
defensive temporaries, as discussed in #187348.

The comparison is bitwise (memcmp), not a value comparison, so unmodified
NaN elements and derived-type padding do not produce spurious stores. The
helpers mirror the ShallowCopy* type/rank specialization structure so the
per-element comparison and copy inline to fixed-size operations.
---
 .../include/flang-rt/runtime/environment.h    |   4 +
 flang-rt/include/flang-rt/runtime/tools.h     |  11 ++
 flang-rt/lib/runtime/assign.cpp               |  15 ++-
 flang-rt/lib/runtime/environment.cpp          |  13 ++
 flang-rt/lib/runtime/tools.cpp                | 122 ++++++++++++++++++
 flang-rt/unittests/Runtime/Assign.cpp         | 122 ++++++++++++++++++
 flang/docs/RuntimeEnvironment.md              |  11 ++
 7 files changed, 297 insertions(+), 1 deletion(-)

diff --git a/flang-rt/include/flang-rt/runtime/environment.h b/flang-rt/include/flang-rt/runtime/environment.h
index 8ae441c12b3f8..a4dabb2ca20fb 100644
--- a/flang-rt/include/flang-rt/runtime/environment.h
+++ b/flang-rt/include/flang-rt/runtime/environment.h
@@ -74,6 +74,10 @@ struct ExecutionEnvironment {
   bool checkPointerDeallocation{true}; // FORT_CHECK_POINTER_DEALLOCATION
   bool truncateStream{true}; // FORT_TRUNCATE_STREAM
   bool noEmptyAllocation{false}; // FORT_NO_EMPTY_ALLOCATION
+  // FLANG_RT_COPYOUT_MODIFIED_ONLY=0 restores the unconditional copy-out
+  // of argument temporaries (CopyOutAssign copies every element back
+  // instead of only the elements the callee modified).
+  bool copyOutModifiedOnly{true}; // FLANG_RT_COPYOUT_MODIFIED_ONLY
 
   enum InternalDebugging { WorkQueue = 1 };
   int internalDebugging{0}; // FLANG_RT_DEBUG
diff --git a/flang-rt/include/flang-rt/runtime/tools.h b/flang-rt/include/flang-rt/runtime/tools.h
index a45c2ac98f2fa..411e5b2221702 100644
--- a/flang-rt/include/flang-rt/runtime/tools.h
+++ b/flang-rt/include/flang-rt/runtime/tools.h
@@ -525,6 +525,17 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from,
     bool toIsContiguous, bool fromIsContiguous);
 RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from);
 
+// Copies payload data like ShallowCopy(), but stores only to the elements of
+// 'to' whose bit patterns differ from the corresponding elements of 'from'.
+// Used by copy-out to avoid stores when the callee never modified the data,
+// so that an original that lives in read-only memory (e.g. a named constant)
+// is not written to by an unmodifying copy-out. The comparison is bitwise, so
+// it is exact when 'from' was originally produced from 'to' by ShallowCopy()
+// (as CopyInAssign() does): unmodified elements compare equal even for NaNs
+// and padding bytes, which a value comparison would misjudge.
+RT_API_ATTRS void ShallowCopyModifiedElements(
+    const Descriptor &to, const Descriptor &from);
+
 // Ensures that a character string is null-terminated, allocating a /p length +1
 // size memory for null-terminator if necessary. Returns the original or a newly
 // allocated null-terminated string (responsibility for deallocation is on the
diff --git a/flang-rt/lib/runtime/assign.cpp b/flang-rt/lib/runtime/assign.cpp
index 2fc584d2569e7..1f7e7c688875f 100644
--- a/flang-rt/lib/runtime/assign.cpp
+++ b/flang-rt/lib/runtime/assign.cpp
@@ -10,6 +10,7 @@
 #include "flang-rt/runtime/assign-impl.h"
 #include "flang-rt/runtime/derived.h"
 #include "flang-rt/runtime/descriptor.h"
+#include "flang-rt/runtime/environment.h"
 #include "flang-rt/runtime/memory.h"
 #include "flang-rt/runtime/stat.h"
 #include "flang-rt/runtime/terminator.h"
@@ -838,8 +839,20 @@ void RTDEF(CopyOutAssign)(
   Terminator terminator{sourceFile, sourceLine};
   // Copyout from the temporary must not cause any finalizations
   // for LHS. The variable must be properly initialized already.
+  // Copy back only the elements that were modified through the temporary:
+  // the temporary was created as a bitwise copy of the variable (see
+  // CopyInAssign above), so an element the callee never assigned is still
+  // bit-identical to the original and must not be stored to. This keeps a
+  // compiler-generated copy-out from writing into read-only storage when the
+  // effective argument is not definable (e.g., a named constant) and the
+  // callee, conformingly, never modified it.
+  // FLANG_RT_COPYOUT_MODIFIED_ONLY=0 restores the unconditional copy-out.
   if (var) {
-    ShallowCopy(*var, temp);
+    if (executionEnvironment.copyOutModifiedOnly) {
+      ShallowCopyModifiedElements(*var, temp);
+    } else {
+      ShallowCopy(*var, temp);
+    }
   }
   temp.Deallocate();
 }
diff --git a/flang-rt/lib/runtime/environment.cpp b/flang-rt/lib/runtime/environment.cpp
index ea98eb4634293..fc19c8c3973e7 100644
--- a/flang-rt/lib/runtime/environment.cpp
+++ b/flang-rt/lib/runtime/environment.cpp
@@ -205,6 +205,19 @@ void ExecutionEnvironment::Configure(int ac, const char *av[],
     }
   }
 
+  if (auto *x{std::getenv("FLANG_RT_COPYOUT_MODIFIED_ONLY")}) {
+    char *end;
+    auto n{std::strtol(x, &end, 10)};
+    if (n >= 0 && n <= 1 && *end == '\0') {
+      copyOutModifiedOnly = n != 0;
+    } else {
+      std::fprintf(stderr,
+          "Fortran runtime: FLANG_RT_COPYOUT_MODIFIED_ONLY=%s is invalid; "
+          "ignored\n",
+          x);
+    }
+  }
+
   if (auto *x{std::getenv("FLANG_RT_DEBUG")}) {
     internalDebugging = std::strtol(x, nullptr, 10);
   }
diff --git a/flang-rt/lib/runtime/tools.cpp b/flang-rt/lib/runtime/tools.cpp
index b7408c81f83f4..56890770c253f 100644
--- a/flang-rt/lib/runtime/tools.cpp
+++ b/flang-rt/lib/runtime/tools.cpp
@@ -271,6 +271,128 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from) {
   ShallowCopy(to, from, to.IsContiguous(), from.IsContiguous());
 }
 
+// Compares one element bitwise and copies it only when it differs. As in the
+// ShallowCopy* helpers above, the compile-time element size lets the compiler
+// inline both the comparison and the copy.
+template <typename P>
+static inline RT_API_ATTRS void CopyElementIfModified(
+    char *toAt, const char *fromAt, std::size_t elementBytes) {
+  constexpr std::size_t typeElementBytes{sizeof(P)};
+  if constexpr (typeElementBytes != 1) {
+    if (runtime::memcmp(toAt, fromAt, typeElementBytes) != 0) {
+      runtime::memcpy(toAt, fromAt, typeElementBytes);
+    }
+  } else {
+    if (runtime::memcmp(toAt, fromAt, elementBytes) != 0) {
+      runtime::memcpy(toAt, fromAt, elementBytes);
+    }
+  }
+}
+
+template <typename P, int RANK = -1>
+static RT_API_ATTRS void ShallowCopyModifiedInner(const Descriptor &to,
+    const Descriptor &from, bool toIsContiguous, bool fromIsContiguous) {
+  std::size_t elementBytes{to.ElementBytes()};
+  if (toIsContiguous) {
+    char *toAt{to.OffsetElement()};
+    if (fromIsContiguous) {
+      const char *fromAt{from.OffsetElement()};
+      for (std::size_t n{to.Elements()}; n-- > 0;
+          toAt += elementBytes, fromAt += elementBytes) {
+        CopyElementIfModified<P>(toAt, fromAt, elementBytes);
+      }
+    } else {
+      DescriptorIterator<RANK> fromIt{from};
+      for (std::size_t n{to.Elements()}; n-- > 0;
+          toAt += elementBytes, fromIt.Advance()) {
+        CopyElementIfModified<P>(
+            toAt, fromIt.template Get<char>(), elementBytes);
+      }
+    }
+  } else {
+    DescriptorIterator<RANK> toIt{to};
+    if (fromIsContiguous) {
+      const char *fromAt{from.OffsetElement()};
+      for (std::size_t n{to.Elements()}; n-- > 0;
+          toIt.Advance(), fromAt += elementBytes) {
+        CopyElementIfModified<P>(
+            toIt.template Get<char>(), fromAt, elementBytes);
+      }
+    } else {
+      DescriptorIterator<RANK> fromIt{from};
+      for (std::size_t n{to.Elements()}; n-- > 0;
+          toIt.Advance(), fromIt.Advance()) {
+        CopyElementIfModified<P>(toIt.template Get<char>(),
+            fromIt.template Get<char>(), elementBytes);
+      }
+    }
+  }
+}
+
+template <typename P>
+static RT_API_ATTRS void ShallowCopyModifiedRank(const Descriptor &to,
+    const Descriptor &from, bool toIsContiguous, bool fromIsContiguous) {
+  INTERNAL_CHECK(to.rank() == from.rank());
+  // Mirror ShallowCopyRank's rank specialization policy.
+  switch (to.rank()) {
+  case 1:
+    ShallowCopyModifiedInner<P, 1>(to, from, toIsContiguous, fromIsContiguous);
+    return;
+  case 2:
+    ShallowCopyModifiedInner<P, 2>(to, from, toIsContiguous, fromIsContiguous);
+    return;
+  case 3:
+    ShallowCopyModifiedInner<P, 3>(to, from, toIsContiguous, fromIsContiguous);
+    return;
+  case 4:
+    ShallowCopyModifiedInner<P, 4>(to, from, toIsContiguous, fromIsContiguous);
+    return;
+  default:
+    ShallowCopyModifiedInner<P>(to, from, toIsContiguous, fromIsContiguous);
+    return;
+  }
+}
+
+RT_API_ATTRS void ShallowCopyModifiedElements(
+    const Descriptor &to, const Descriptor &from) {
+  bool toIsContiguous{to.IsContiguous()};
+  bool fromIsContiguous{from.IsContiguous()};
+  std::size_t elementBytes{to.ElementBytes()};
+  // Same type-based dispatch as ShallowCopy() above, so the per-element
+  // comparison and copy inline to fixed-size operations.
+  if (to.type().IsInteger()) {
+    if (elementBytes == sizeof(int64_t)) {
+      ShallowCopyModifiedRank<int64_t>(
+          to, from, toIsContiguous, fromIsContiguous);
+    } else if (elementBytes == sizeof(int32_t)) {
+      ShallowCopyModifiedRank<int32_t>(
+          to, from, toIsContiguous, fromIsContiguous);
+    } else if (elementBytes == sizeof(int16_t)) {
+      ShallowCopyModifiedRank<int16_t>(
+          to, from, toIsContiguous, fromIsContiguous);
+#if defined USING_NATIVE_INT128_T
+    } else if (elementBytes == sizeof(__int128_t)) {
+      ShallowCopyModifiedRank<__int128_t>(
+          to, from, toIsContiguous, fromIsContiguous);
+#endif
+    } else {
+      ShallowCopyModifiedRank<char>(to, from, toIsContiguous, fromIsContiguous);
+    }
+  } else if (to.type().IsReal()) {
+    if (elementBytes == sizeof(double)) {
+      ShallowCopyModifiedRank<double>(
+          to, from, toIsContiguous, fromIsContiguous);
+    } else if (elementBytes == sizeof(float)) {
+      ShallowCopyModifiedRank<float>(
+          to, from, toIsContiguous, fromIsContiguous);
+    } else {
+      ShallowCopyModifiedRank<char>(to, from, toIsContiguous, fromIsContiguous);
+    }
+  } else {
+    ShallowCopyModifiedRank<char>(to, from, toIsContiguous, fromIsContiguous);
+  }
+}
+
 RT_API_ATTRS char *EnsureNullTerminated(
     char *str, std::size_t length, Terminator &terminator) {
   if (runtime::memchr(str, '\0', length) == nullptr) {
diff --git a/flang-rt/unittests/Runtime/Assign.cpp b/flang-rt/unittests/Runtime/Assign.cpp
index 8e9b89d7f661f..e9b8de52ffde2 100644
--- a/flang-rt/unittests/Runtime/Assign.cpp
+++ b/flang-rt/unittests/Runtime/Assign.cpp
@@ -10,7 +10,14 @@
 #include "CrashHandlerFixture.h"
 #include "tools.h"
 #include "gtest/gtest.h"
+#include "flang-rt/runtime/environment.h"
+#include <cstdint>
+#include <cstring>
 #include <vector>
+#if defined(__unix__) || defined(__APPLE__)
+#include <sys/mman.h>
+#include <unistd.h>
+#endif
 
 using namespace Fortran::runtime;
 using Fortran::common::TypeCategory;
@@ -446,3 +453,118 @@ TEST(AssignSimpleCrash, NonAllocatableElementCountMismatch) {
   ASSERT_DEATH(RTNAME(AssignSimple)(dest, source, __FILE__, __LINE__),
       "AssignSimple: mismatching element counts");
 }
+
+TEST(Assign, RTNAME(CopyOutAssign)) {
+  // Copy-out writes back the elements the callee modified through the
+  // temporary and performs no stores for the elements it never touched.
+  // Discontiguous var: stride-2 view (elements 1,3,5,7) of an 8-element
+  // backing array, as copy-in/copy-out creates for a non-contiguous actual.
+  int data[8] = {1, 2, 3, 4, 5, 6, 7, 8};
+  TypeCode intType{TypeCategory::Integer, 4};
+  StaticDescriptor<1> staticVar;
+  Descriptor &var{staticVar.descriptor()};
+  SubscriptValue extent[1]{4};
+  var.Establish(intType, sizeof(int), data, 1, extent);
+  var.GetDimension(0).SetLowerBound(1);
+  var.GetDimension(0).SetByteStride(sizeof(int) * 2);
+
+  StaticDescriptor<1> staticTemp;
+  Descriptor &temp{staticTemp.descriptor()};
+  RTNAME(CopyInAssign)(temp, var, __FILE__, __LINE__);
+  ASSERT_TRUE(temp.IsAllocated());
+  ASSERT_TRUE(temp.IsContiguous());
+
+  // The "callee" modifies the first and third elements of the temporary.
+  *temp.OffsetElement<int>(0 * sizeof(int)) = 100;
+  *temp.OffsetElement<int>(2 * sizeof(int)) = 300;
+
+  RTNAME(CopyOutAssign)(&var, temp, __FILE__, __LINE__);
+
+  int expected[8] = {100, 2, 3, 4, 300, 6, 7, 8};
+  EXPECT_EQ(std::memcmp(data, expected, 8 * sizeof(int)), 0);
+}
+
+#if defined(__unix__) || defined(__APPLE__)
+TEST(Assign, RTNAME(CopyOutAssignReadOnlyUnmodified)) {
+  // An unmodified copy-out must perform no stores at all: the original may
+  // live in read-only memory (e.g. a named constant's storage). The array
+  // includes a NaN element to verify that the comparison is bitwise — a
+  // value comparison would consider the unmodified NaN element "changed"
+  // and store to it, faulting on the read-only page.
+  std::size_t pageSize{static_cast<std::size_t>(sysconf(_SC_PAGESIZE))};
+  void *page{mmap(nullptr, pageSize, PROT_READ | PROT_WRITE,
+      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0)};
+  ASSERT_NE(page, MAP_FAILED);
+  double *data{static_cast<double *>(page)};
+  for (int j{0}; j < 8; ++j) {
+    data[j] = j + 1;
+  }
+  std::uint64_t quietNaN{0x7FF8000000000000ULL};
+  std::memcpy(&data[2], &quietNaN, sizeof(double));
+  ASSERT_EQ(mprotect(page, pageSize, PROT_READ), 0);
+
+  // Discontiguous read-only var: stride-2 view (elements 1,NaN,5,7).
+  StaticDescriptor<1> staticVar;
+  Descriptor &var{staticVar.descriptor()};
+  SubscriptValue extent[1]{4};
+  var.Establish(
+      TypeCode{TypeCategory::Real, 8}, sizeof(double), data, 1, extent);
+  var.GetDimension(0).SetLowerBound(1);
+  var.GetDimension(0).SetByteStride(sizeof(double) * 2);
+
+  StaticDescriptor<1> staticTemp;
+  Descriptor &temp{staticTemp.descriptor()};
+  RTNAME(CopyInAssign)(temp, var, __FILE__, __LINE__);
+  ASSERT_TRUE(temp.IsAllocated());
+
+  // The "callee" only reads the temporary; copying out into the read-only
+  // original must not fault.
+  RTNAME(CopyOutAssign)(&var, temp, __FILE__, __LINE__);
+
+  std::uint64_t elem2Bits;
+  std::memcpy(&elem2Bits, &data[2], sizeof(double));
+  EXPECT_EQ(elem2Bits, quietNaN);
+  EXPECT_EQ(data[0], 1.0);
+  EXPECT_EQ(data[4], 5.0);
+  ASSERT_EQ(munmap(page, pageSize), 0);
+}
+#endif
+
+#if defined(__unix__) || defined(__APPLE__)
+TEST(Assign, RTNAME(CopyOutAssignUnconditionalEnvVar)) {
+  // With FLANG_RT_COPYOUT_MODIFIED_ONLY=0 semantics (unconditional copy-out),
+  // even an unmodified copy-out stores every element, so a read-only original
+  // faults. This proves the environment control selects the legacy path.
+  std::size_t pageSize{static_cast<std::size_t>(sysconf(_SC_PAGESIZE))};
+  void *page{mmap(nullptr, pageSize, PROT_READ | PROT_WRITE,
+      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0)};
+  ASSERT_NE(page, MAP_FAILED);
+  double *data{static_cast<double *>(page)};
+  for (int j{0}; j < 8; ++j) {
+    data[j] = j + 1;
+  }
+  ASSERT_EQ(mprotect(page, pageSize, PROT_READ), 0);
+
+  StaticDescriptor<1> staticVar;
+  Descriptor &var{staticVar.descriptor()};
+  SubscriptValue extent[1]{4};
+  var.Establish(
+      TypeCode{TypeCategory::Real, 8}, sizeof(double), data, 1, extent);
+  var.GetDimension(0).SetLowerBound(1);
+  var.GetDimension(0).SetByteStride(sizeof(double) * 2);
+
+  StaticDescriptor<1> staticTemp;
+  Descriptor &temp{staticTemp.descriptor()};
+  RTNAME(CopyInAssign)(temp, var, __FILE__, __LINE__);
+  ASSERT_TRUE(temp.IsAllocated());
+
+  executionEnvironment.copyOutModifiedOnly = false;
+  EXPECT_DEATH(RTNAME(CopyOutAssign)(&var, temp, __FILE__, __LINE__), "");
+  executionEnvironment.copyOutModifiedOnly = true;
+
+  // The parent's temp is still allocated (the death happened in the child);
+  // clean it up through the default path.
+  RTNAME(CopyOutAssign)(&var, temp, __FILE__, __LINE__);
+  ASSERT_EQ(munmap(page, pageSize), 0);
+}
+#endif
diff --git a/flang/docs/RuntimeEnvironment.md b/flang/docs/RuntimeEnvironment.md
index 0414ea1e0d59c..a24da8e6a6a62 100644
--- a/flang/docs/RuntimeEnvironment.md
+++ b/flang/docs/RuntimeEnvironment.md
@@ -34,6 +34,17 @@ Determines data conversions applied to unformatted I/O.
 * `BIG_ENDIAN`: assume input is big-endian; emit big-endian output
 * `SWAP`: reverse endianness (always convert)
 
+## `FLANG_RT_COPYOUT_MODIFIED_ONLY`
+
+When the compiler passes a copy of an actual argument to a procedure
+(copy-in/copy-out), the runtime copies back only the elements whose bit
+patterns were changed through the temporary copy. This avoids stores to
+the original argument when the callee never modified the data -- in
+particular, stores into read-only storage backing a non-definable actual
+argument.
+Set `FLANG_RT_COPYOUT_MODIFIED_ONLY=0` to restore the unconditional
+copy-out of every element.
+
 ## `FORT_CHECK_POINTER_DEALLOCATION`
 
 Fortran requires that a pointer that appears in a `DEALLOCATE` statement



More information about the flang-commits mailing list