[flang-commits] [flang] [flang] Skip optimized bufferization on volatile refs (PR #140781)

[Asher Mancinelli via flang-commits]

https://github.com/ashermancinelli updated https://github.com/llvm/llvm-project/pull/140781

>From f84c4cc89bfbdb233b17d2b5f85ec73e90e46529 Mon Sep 17 00:00:00 2001
From: Asher Mancinelli <ashermancinelli at gmail.com>
Date: Mon, 19 May 2025 19:43:15 -0700
Subject: [PATCH 1/2] [flang] Skip optimized bufferization on volatile refs

Memory effects on the volatile memory resource may not be attached
to a particular source, in which case the value of an effect will
be null which caused this test case to crash in the optimized
bufferization pass's safety analysis because it assumes it can get
the SSA value modified by the memory effect. This is because memory
effects on the volatile resource indicate that the operation must
not be reordered with respect to other volatile operations, but
there is not a material ssa value that can be pointed to.

This patch changes the safety checks to indicate that that memory
effects on volatile resources are not safe for optimized bufferization.
---
 .../Transforms/OptimizedBufferization.cpp     |  7 +++
 .../HLFIR/opt-bufferization-skip-volatile.fir | 49 +++++++++++++++++++
 2 files changed, 56 insertions(+)
 create mode 100644 flang/test/HLFIR/opt-bufferization-skip-volatile.fir

diff --git a/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp b/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
index 2f6ee2592a84f..120dc4c51f202 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
@@ -608,6 +608,13 @@ ElementalAssignBufferization::findMatch(hlfir::ElementalOp elemental) {
       return std::nullopt;
     }
 
+    // Don't allow any reads to or writes from volatile memory
+    if (mlir::isa<mlir::MemoryEffects::Read, mlir::MemoryEffects::Write>(
+            effect.getEffect()) &&
+        mlir::isa<fir::VolatileMemoryResource>(effect.getResource())) {
+      return std::nullopt;
+    }
+
     // allow if and only if the reads are from the elemental indices, in order
     // => each iteration doesn't read values written by other iterations
     // don't allow reads from a different value which may alias: fir alias
diff --git a/flang/test/HLFIR/opt-bufferization-skip-volatile.fir b/flang/test/HLFIR/opt-bufferization-skip-volatile.fir
new file mode 100644
index 0000000000000..158f92bf207d2
--- /dev/null
+++ b/flang/test/HLFIR/opt-bufferization-skip-volatile.fir
@@ -0,0 +1,49 @@
+// RUN: fir-opt --pass-pipeline="builtin.module(func.func(opt-bufferization))" %s | FileCheck %s
+
+// Ensure optimized bufferization preserves the semantics of volatile arrays
+func.func @minimal_volatile_test() {
+  %c1 = arith.constant 1 : index
+  %c200 = arith.constant 200 : index
+  
+  // Create a volatile array
+  %1 = fir.address_of(@_QMtestEarray) : !fir.ref<!fir.array<200xf32>>
+  %2 = fir.shape %c200 : (index) -> !fir.shape<1>
+  %3 = fir.volatile_cast %1 : (!fir.ref<!fir.array<200xf32>>) -> !fir.ref<!fir.array<200xf32>, volatile>
+  %4:2 = hlfir.declare %3(%2) {fortran_attrs = #fir.var_attrs<volatile>, uniq_name = "_QMtestEarray"} : (!fir.ref<!fir.array<200xf32>, volatile>, !fir.shape<1>) -> (!fir.ref<!fir.array<200xf32>, volatile>, !fir.ref<!fir.array<200xf32>, volatile>)
+  
+  // Create an elemental operation that negates each element
+  %5 = hlfir.elemental %2 unordered : (!fir.shape<1>) -> !hlfir.expr<200xf32> {
+  ^bb0(%arg1: index):
+    %6 = hlfir.designate %4#0 (%arg1) : (!fir.ref<!fir.array<200xf32>, volatile>, index) -> !fir.ref<f32, volatile>
+    %7 = fir.load %6 : !fir.ref<f32, volatile>
+    %8 = arith.negf %7 : f32
+    hlfir.yield_element %8 : f32
+  }
+  
+  // Assign the result back to the volatile array
+  hlfir.assign %5 to %4#0 : !hlfir.expr<200xf32>, !fir.ref<!fir.array<200xf32>, volatile>
+  hlfir.destroy %5 : !hlfir.expr<200xf32>
+  
+  return
+}
+
+fir.global @_QMtestEarray : !fir.array<200xf32> 
+
+// CHECK-LABEL:   func.func @minimal_volatile_test() {
+// CHECK:           %[[VAL_0:.*]] = arith.constant 200 : index
+// CHECK:           %[[VAL_1:.*]] = fir.address_of(@_QMtestEarray) : !fir.ref<!fir.array<200xf32>>
+// CHECK:           %[[VAL_2:.*]] = fir.shape %[[VAL_0]] : (index) -> !fir.shape<1>
+// CHECK:           %[[VAL_3:.*]] = fir.volatile_cast %[[VAL_1]] : (!fir.ref<!fir.array<200xf32>>) -> !fir.ref<!fir.array<200xf32>, volatile>
+// CHECK:           %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_3]](%[[VAL_2]]) {fortran_attrs = #fir.var_attrs<volatile>, uniq_name = "_QMtestEarray"} : (!fir.ref<!fir.array<200xf32>, volatile>, !fir.shape<1>) -> (!fir.ref<!fir.array<200xf32>, volatile>, !fir.ref<!fir.array<200xf32>, volatile>)
+// CHECK:           %[[VAL_5:.*]] = hlfir.elemental %[[VAL_2]] unordered : (!fir.shape<1>) -> !hlfir.expr<200xf32> {
+// CHECK:           ^bb0(%[[VAL_6:.*]]: index):
+// CHECK:             %[[VAL_7:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_6]])  : (!fir.ref<!fir.array<200xf32>, volatile>, index) -> !fir.ref<f32, volatile>
+// CHECK:             %[[VAL_8:.*]] = fir.load %[[VAL_7]] : !fir.ref<f32, volatile>
+// CHECK:             %[[VAL_9:.*]] = arith.negf %[[VAL_8]] : f32
+// CHECK:             hlfir.yield_element %[[VAL_9]] : f32
+// CHECK:           }
+// CHECK:           hlfir.assign %[[VAL_5]] to %[[VAL_4]]#0 : !hlfir.expr<200xf32>, !fir.ref<!fir.array<200xf32>, volatile>
+// CHECK:           hlfir.destroy %[[VAL_5]] : !hlfir.expr<200xf32>
+// CHECK:           return
+// CHECK:         }
+// CHECK:         fir.global @_QMtestEarray : !fir.array<200xf32>

>From 8b8ff482c48b157b55e95d2a6ae6a88ef0f05929 Mon Sep 17 00:00:00 2001
From: Asher Mancinelli <ashermancinelli at gmail.com>
Date: Tue, 20 May 2025 12:32:53 -0700
Subject: [PATCH 2/2] Check for null effect value instead of volatile resource

---
 .../Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp  | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp b/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
index 120dc4c51f202..e2ca754a1817a 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
@@ -608,10 +608,9 @@ ElementalAssignBufferization::findMatch(hlfir::ElementalOp elemental) {
       return std::nullopt;
     }
 
-    // Don't allow any reads to or writes from volatile memory
-    if (mlir::isa<mlir::MemoryEffects::Read, mlir::MemoryEffects::Write>(
-            effect.getEffect()) &&
-        mlir::isa<fir::VolatileMemoryResource>(effect.getResource())) {
+    if (effect.getValue() == nullptr) {
+      LLVM_DEBUG(llvm::dbgs()
+                 << "side-effect with no value, cannot analyze further\n");
       return std::nullopt;
     }