[flang-commits] [flang] [flang] Promote scalar slots reached through fir.convert and fir.declare (PR #219314)
Vijay Kandiah via flang-commits
flang-commits at lists.llvm.org
Mon Aug 31 08:54:25 PDT 2026
https://github.com/VijayKandiah updated https://github.com/llvm/llvm-project/pull/219314
>From bb27d3eab08436c0f0288640c755c40a54a9fc63 Mon Sep 17 00:00:00 2001
From: Vijay Kandiah <vkandiah at nvidia.com>
Date: Thu, 27 Aug 2026 15:04:34 -0700
Subject: [PATCH 1/3] [flang][FIR] Promote scalar slots reached through
fir.convert and fir.declare
---
.../include/flang/Optimizer/Dialect/FIROps.td | 6 +
flang/lib/Optimizer/Dialect/FIROps.cpp | 164 ++++++++++++-
flang/test/Fir/mem2reg.mlir | 231 ++++++++++++++++--
3 files changed, 374 insertions(+), 27 deletions(-)
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index fd0b5b16c87ad..8651728acd09b 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -2750,6 +2750,10 @@ def fir_VolatileCastOp
def fir_ConvertOp
: fir_SimpleOneResultOp<"convert", [NoMemoryEffect, ViewLikeOpInterface,
ConditionallySpeculatable,
+ DeclareOpInterfaceMethods<
+ PromotableOpInterface>,
+ DeclareOpInterfaceMethods<
+ PromotableAliaserInterface>,
fir_FortranObjectViewOpInterface]> {
let summary = "encapsulates all Fortran entity type conversions";
@@ -3283,6 +3287,8 @@ def fir_DeclareOp
DeclareOpInterfaceMethods<PromotableOpInterface,
["requiresReplacedValues",
"visitReplacedValues"]>,
+ DeclareOpInterfaceMethods<
+ PromotableAliaserInterface>,
fir_FortranObjectViewOpInterface]> {
let summary = "declare a variable";
diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp
index 108f7fc793c61..02bcec4a520c9 100644
--- a/flang/lib/Optimizer/Dialect/FIROps.cpp
+++ b/flang/lib/Optimizer/Dialect/FIROps.cpp
@@ -33,6 +33,7 @@
#include "mlir/IR/PatternMatch.h"
#include "mlir/IR/TypeRange.h"
#include "mlir/Interfaces/DataLayoutInterfaces.h"
+#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/TypeSwitch.h"
@@ -2135,6 +2136,131 @@ llvm::LogicalResult fir::ConvertOp::verify() {
<< getValue().getType() << " / " << getType();
}
+/// Pointee of a reference to a simple scalar, or null. Both fir.ref and rank-0
+/// memref qualify, since the storage of a scalar is cast between those forms.
+static mlir::Type getScalarSlotPointeeType(mlir::Type type) {
+ mlir::Type eleTy;
+ if (auto refTy = mlir::dyn_cast<fir::ReferenceType>(type)) {
+ eleTy = refTy.getEleTy();
+ } else if (auto memrefTy = mlir::dyn_cast<mlir::MemRefType>(type)) {
+ // A rank, layout or memory space lets the cast reinterpret the storage.
+ if (memrefTy.getRank() != 0 || !memrefTy.getLayout().isIdentity() ||
+ memrefTy.getMemorySpace())
+ return {};
+ eleTy = memrefTy.getElementType();
+ } else {
+ return {};
+ }
+ if (!mlir::isa<mlir::IntegerType, mlir::FloatType, mlir::ComplexType,
+ fir::LogicalType>(eleTy))
+ return {};
+ return eleTy;
+}
+
+/// The value a cast or declare aliases, or null if `value` is not one of those.
+static mlir::Value getAliasedSlotPointer(mlir::Value value) {
+ mlir::Operation *def = value.getDefiningOp();
+ if (auto convert = mlir::dyn_cast_or_null<fir::ConvertOp>(def))
+ return convert.getValue();
+ if (auto declare = mlir::dyn_cast_or_null<fir::DeclareOp>(def))
+ return declare.getMemref();
+ return {};
+}
+
+/// Checks whether every read of the slot reached through `pointer` has a write
+/// to it earlier in the same block. Otherwise promotion replaces the read with
+/// the allocation's default value, which is fir.undefined for fir.alloca but
+/// poison for a memref allocation, so promoting through a cast would change
+/// what reading an uninitialized variable does.
+static bool everyReadFollowsAWrite(mlir::Value pointer, mlir::Type pointee) {
+ // Start at the root of the chain, to see writes through a sibling alias.
+ while (mlir::Value aliased = getAliasedSlotPointer(pointer)) {
+ if (getScalarSlotPointeeType(aliased.getType()) != pointee)
+ break;
+ pointer = aliased;
+ }
+
+ llvm::SmallVector<mlir::Operation *> reads;
+ llvm::DenseMap<mlir::Block *, mlir::Operation *> firstWrite;
+ llvm::SmallVector<mlir::Value> worklist{pointer};
+ llvm::SmallPtrSet<mlir::Value, 8> visited{pointer};
+ while (!worklist.empty()) {
+ for (mlir::OpOperand &use : worklist.pop_back_val().getUses()) {
+ mlir::Operation *user = use.getOwner();
+ if (mlir::isa<fir::ConvertOp, fir::DeclareOp>(user)) {
+ mlir::Value alias = user->getResult(0);
+ // A different pointee is another slot, which blocks promotion itself.
+ if (getScalarSlotPointeeType(alias.getType()) == pointee &&
+ visited.insert(alias).second)
+ worklist.push_back(alias);
+ continue;
+ }
+ // A user that is not a promotable access blocks promotion itself.
+ if (auto memOp = mlir::dyn_cast<mlir::PromotableMemOpInterface>(user)) {
+ mlir::MemorySlot slot{use.get(), pointee};
+ if (memOp.storesTo(slot)) {
+ mlir::Operation *&earliest = firstWrite[user->getBlock()];
+ if (!earliest || user->isBeforeInBlock(earliest))
+ earliest = user;
+ } else if (memOp.loadsFrom(slot)) {
+ reads.push_back(user);
+ }
+ }
+ }
+ }
+
+ return llvm::all_of(reads, [&firstWrite](mlir::Operation *read) {
+ auto write = firstWrite.find(read->getBlock());
+ return write != firstWrite.end() && write->second->isBeforeInBlock(read);
+ });
+}
+
+/// The pointee a cast carries through unchanged, or null if the cast is not a
+/// transparent alias of the slot.
+static mlir::Type getPointeePreservedByCast(fir::ConvertOp op) {
+ mlir::Type fromTy = op.getValue().getType();
+ mlir::Type toTy = op.getType();
+ // Volatility may be added or dropped, and such accesses must be kept.
+ if (fir::isa_volatile_type(fromTy) || fir::isa_volatile_type(toTy))
+ return {};
+ // A scalar slot on both sides also excludes fir.llvm_ptr and llvm.ptr, which
+ // carry an address space.
+ mlir::Type from = getScalarSlotPointeeType(fromTy);
+ mlir::Type to = getScalarSlotPointeeType(toTy);
+ if (!from || from != to)
+ return {};
+ // A read that no write reaches must keep its access.
+ if (!everyReadFollowsAWrite(op.getValue(), to))
+ return {};
+ return to;
+}
+
+void fir::ConvertOp::getPromotableSlotAliases(
+ mlir::OpOperand &aliasedSlotPointerOperand,
+ const mlir::MemorySlot &parentSlot,
+ llvm::SmallVectorImpl<mlir::MemorySlot> &newMemorySlots) {
+ // The result aliases the slot exactly, so value projections are unchanged.
+ if (mlir::Type pointee = getPointeePreservedByCast(*this))
+ newMemorySlots.push_back({getResult(), pointee});
+}
+
+bool fir::ConvertOp::canUsesBeRemoved(
+ const mlir::SmallPtrSetImpl<mlir::OpOperand *> &blockingUses,
+ mlir::SmallVectorImpl<mlir::OpOperand *> &newBlockingUses,
+ const mlir::DataLayout &dataLayout) {
+ if (!getPointeePreservedByCast(*this))
+ return false;
+ for (mlir::OpOperand &use : getResult().getUses())
+ newBlockingUses.push_back(&use);
+ return true;
+}
+
+mlir::DeletionKind fir::ConvertOp::removeBlockingUses(
+ const mlir::SmallPtrSetImpl<mlir::OpOperand *> &blockingUses,
+ mlir::OpBuilder &builder) {
+ return mlir::DeletionKind::Delete;
+}
+
mlir::Speculation::Speculatability fir::ConvertOp::getSpeculatability() {
// fir.convert is speculatable, in general. The only concern may be
// converting from or/and to floating point types, which may trigger
@@ -6038,23 +6164,39 @@ llvm::LogicalResult fir::DeclareOp::verify() {
return fortranVar.verifyDeclareLikeOpImpl(getMemref());
}
+/// Return the pointee of the storage a fir.declare denotes, or null if that
+/// storage must not be promoted.
+static mlir::Type getPromotablePointeeOfDeclare(fir::DeclareOp op) {
+ // Accesses through a volatile reference have to be preserved.
+ if (fir::isa_volatile_type(op.getType()))
+ return {};
+ mlir::Type pointee = fir::unwrapRefType(op.getType());
+ if (!isLegalTypeForValueDeclare(pointee))
+ return {};
+ // Values are not converted between the slot and its alias, so the pointee has
+ // to be the same on both sides.
+ if (fir::unwrapRefType(op.getMemref().getType()) != pointee)
+ return {};
+ return pointee;
+}
+
+void fir::DeclareOp::getPromotableSlotAliases(
+ mlir::OpOperand &aliasedSlotPointerOperand,
+ const mlir::MemorySlot &parentSlot,
+ llvm::SmallVectorImpl<mlir::MemorySlot> &newMemorySlots) {
+ // fir.declare only attaches source information; the storage is the same.
+ if (mlir::Type pointee = getPromotablePointeeOfDeclare(*this))
+ newMemorySlots.push_back({getResult(), pointee});
+}
+
bool fir::DeclareOp::canUsesBeRemoved(
const mlir::SmallPtrSetImpl<mlir::OpOperand *> &blockingUses,
mlir::SmallVectorImpl<mlir::OpOperand *> &newBlockingUses,
const mlir::DataLayout &dataLayout) {
- if (!isLegalTypeForValueDeclare(fir::unwrapRefType(getType())))
+ if (!getPromotablePointeeOfDeclare(*this))
return false;
- // MLIR's mem2reg computes defining blocks only from direct users of
- // the slot pointer. Stores through fir.declare are not direct users,
- // so they are not registered as defining blocks. This causes missing
- // phi nodes at join points (e.g., loop headers). Restrict promotion
- // to the single-block case where no phi nodes are needed.
- mlir::Block *declBlock = getOperation()->getBlock();
- for (mlir::OpOperand &use : getResult().getUses()) {
- if (use.getOwner()->getBlock() != declBlock)
- return false;
+ for (mlir::OpOperand &use : getResult().getUses())
newBlockingUses.push_back(&use);
- }
return true;
}
diff --git a/flang/test/Fir/mem2reg.mlir b/flang/test/Fir/mem2reg.mlir
index 154580c626e7c..8dee16102363c 100644
--- a/flang/test/Fir/mem2reg.mlir
+++ b/flang/test/Fir/mem2reg.mlir
@@ -158,17 +158,17 @@ func.func @box_not_mem2reg(%arg0: !fir.ref<!fir.box<f32>> {fir.bindc_name = "i"}
// -----
-// Conditional store in a different block through fir.declare is not promoted
-// because MLIR mem2reg would not place the needed phi nodes correctly.
+// Write in another block than the fir.declare: found through the alias, so the
+// join point gets a block argument.
// CHECK-LABEL: func.func @block_argument_value(
// CHECK-SAME: %[[ARG0:.*]]: i32,
// CHECK-SAME: %[[ARG1:.*]]: i1) -> i32 {
-// CHECK: fir.alloca i32
-// CHECK: fir.declare
-// CHECK: fir.store
-// CHECK: fir.store
-// CHECK: fir.load
+// CHECK-NOT: fir.alloca
+// CHECK: llvm.cond_br %[[ARG1]], ^bb1, ^bb2(%[[C42:.*]] : i32)
+// CHECK: llvm.br ^bb2(%[[ARG0]] : i32)
+// CHECK: ^bb2(%[[PHI:.*]]: i32):
+// CHECK: return %[[PHI]] : i32
func.func @block_argument_value(%arg0: i32, %cdt: i1) -> i32 {
%c42_i32 = arith.constant 42 : i32
%3 = fir.alloca i32 {bindc_name = "jlocal", uniq_name = "_QFfooEjlocal"}
@@ -185,19 +185,17 @@ func.func @block_argument_value(%arg0: i32, %cdt: i1) -> i32 {
// -----
-// Conditional store inside a loop through fir.declare must not be promoted.
-// MLIR's mem2reg does not register stores through declares as defining blocks,
-// so phi nodes at the loop header would be missing, losing the update.
+// Write inside a loop through fir.declare: the header takes a block argument.
// CHECK-LABEL: func.func @loop_conditional_update(
// CHECK-SAME: %[[ARG0:.*]]: i32,
// CHECK-SAME: %[[ARG1:.*]]: i1) -> i32 {
-// CHECK: fir.alloca i32
-// CHECK: fir.declare
-// CHECK: fir.store
-// CHECK: fir.load
-// CHECK: fir.store
-// CHECK: fir.load
+// CHECK-NOT: fir.alloca
+// CHECK: llvm.br ^bb1(%[[ARG0]] : i32)
+// CHECK: ^bb1(%[[PHI:.*]]: i32):
+// CHECK: %[[NEW:.*]] = arith.subi %[[PHI]], %{{.*}} : i32
+// CHECK: llvm.br ^bb1(%[[NEW]] : i32)
+// CHECK: return %[[PHI]] : i32
func.func @loop_conditional_update(%arg0: i32, %cdt: i1) -> i32 {
%c1 = arith.constant 1 : i32
%alloca = fir.alloca i32 {bindc_name = "mywatch", uniq_name = "_QFkernelEmywatch"}
@@ -260,3 +258,204 @@ func.func @dummy_scope_block_argument(%arg : i32, %cond : i1) {
fir.call @use(%result) : (i32) -> ()
return
}
+
+// -----
+
+// CHECK-LABEL: func.func @convert_preserving_pointee(
+// CHECK-NOT: fir.alloca
+func.func @convert_preserving_pointee(%arg : i32) {
+ %alloca = fir.alloca i32
+ %conv = fir.convert %alloca : (!fir.ref<i32>) -> !fir.ref<i32>
+ fir.store %arg to %conv : !fir.ref<i32>
+ %v = fir.load %conv : !fir.ref<i32>
+ fir.call @use(%v) : (i32) -> ()
+ return
+}
+
+// -----
+
+// A cast that changes the pointee is not a transparent alias.
+
+// CHECK-LABEL: func.func @convert_changing_pointee(
+// CHECK: fir.alloca
+func.func @convert_changing_pointee(%arg : i32) {
+ %alloca = fir.alloca i32
+ fir.store %arg to %alloca : !fir.ref<i32>
+ %conv = fir.convert %alloca : (!fir.ref<i32>) -> !fir.ref<f32>
+ %v = fir.load %conv : !fir.ref<f32>
+ fir.call @usef(%v) : (f32) -> ()
+ return
+}
+
+// -----
+
+// A scalar slot reached through both cast directions, as lowering emits it:
+// memref.alloca -> fir.convert -> fir.declare -> fir.convert -> store/load.
+
+// CHECK-LABEL: func.func @scalar_slot_through_casts(
+// CHECK-NOT: memref.alloca
+// CHECK-NOT: fir.declare
+// CHECK: %[[IDX:.*]] = arith.index_cast %{{.*}} : index to i32
+// CHECK: fir.declare_value %[[IDX]]
+// CHECK: "test.use"(%[[IDX]])
+func.func @scalar_slot_through_casts(%n: index) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ scf.parallel (%i) = (%c0) to (%n) step (%c1) {
+ %alloca = memref.alloca() {bindc_name = "k"} : memref<i32>
+ %r = fir.convert %alloca : (memref<i32>) -> !fir.ref<i32>
+ %d = fir.declare %r {uniq_name = "_QFEk"} : (!fir.ref<i32>) -> !fir.ref<i32>
+ %v = arith.index_cast %i : index to i32
+ %m = fir.convert %d : (!fir.ref<i32>) -> memref<i32>
+ memref.store %v, %m[] : memref<i32>
+ %l = memref.load %m[] : memref<i32>
+ "test.use"(%l) : (i32) -> ()
+ scf.reduce
+ }
+ return
+}
+
+// -----
+
+// A ranked memref may be indexed, so the cast is not a transparent alias.
+
+// CHECK-LABEL: func.func @ranked_memref_not_promoted(
+// CHECK: fir.alloca
+func.func @ranked_memref_not_promoted(%arg: i32) {
+ %c0 = arith.constant 0 : index
+ %alloca = fir.alloca i32
+ %m = fir.convert %alloca : (!fir.ref<i32>) -> memref<1xi32>
+ memref.store %arg, %m[%c0] : memref<1xi32>
+ %l = memref.load %m[%c0] : memref<1xi32>
+ "test.use"(%l) : (i32) -> ()
+ return
+}
+
+// -----
+
+// A read that no write reaches would take the default value of the allocation,
+// which is poison here, so the cast must not report itself as an alias.
+
+// CHECK-LABEL: func.func @read_only_slot_not_promoted(
+// CHECK-NOT: ub.poison
+// CHECK: memref.alloca
+// CHECK: memref.load
+func.func @read_only_slot_not_promoted() {
+ %alloca = memref.alloca() {bindc_name = "x"} : memref<i32>
+ %r = fir.convert %alloca : (memref<i32>) -> !fir.ref<i32>
+ %d = fir.declare %r {uniq_name = "_QFEx"} : (!fir.ref<i32>) -> !fir.ref<i32>
+ %m = fir.convert %d : (!fir.ref<i32>) -> memref<i32>
+ %l = memref.load %m[] : memref<i32>
+ "test.use"(%l) : (i32) -> ()
+ return
+}
+
+// -----
+
+// Written on one path only, so no write reaches the read in the entry block.
+
+// CHECK-LABEL: func.func @partially_written_slot_not_promoted(
+// CHECK-NOT: ub.poison
+// CHECK: memref.alloca
+// CHECK: memref.load
+func.func @partially_written_slot_not_promoted(%c: i1, %arg: i32) {
+ %alloca = memref.alloca() {bindc_name = "x"} : memref<i32>
+ %r = fir.convert %alloca : (memref<i32>) -> !fir.ref<i32>
+ %d = fir.declare %r {uniq_name = "_QFEx"} : (!fir.ref<i32>) -> !fir.ref<i32>
+ %m = fir.convert %d : (!fir.ref<i32>) -> memref<i32>
+ scf.if %c {
+ memref.store %arg, %m[] : memref<i32>
+ }
+ %l = memref.load %m[] : memref<i32>
+ "test.use"(%l) : (i32) -> ()
+ return
+}
+
+// -----
+
+// The write is through a different alias than the read, so it is only found by
+// walking to the root of the chain.
+
+// CHECK-LABEL: func.func @store_through_sibling_alias(
+// CHECK-NOT: memref.alloca
+// CHECK: "test.use"(%[[ARG:.*]])
+func.func @store_through_sibling_alias(%arg: i32) {
+ %alloca = memref.alloca() {bindc_name = "x"} : memref<i32>
+ %r = fir.convert %alloca : (memref<i32>) -> !fir.ref<i32>
+ %d = fir.declare %r {uniq_name = "_QFEx"} : (!fir.ref<i32>) -> !fir.ref<i32>
+ %stored = fir.convert %d : (!fir.ref<i32>) -> memref<i32>
+ memref.store %arg, %stored[] : memref<i32>
+ %loaded = fir.convert %d : (!fir.ref<i32>) -> memref<i32>
+ %l = memref.load %loaded[] : memref<i32>
+ "test.use"(%l) : (i32) -> ()
+ return
+}
+
+// -----
+
+// A declare that does not carry the pointee through would need the values of
+// the two slots to be converted, so it is not an alias.
+
+// CHECK-LABEL: func.func @declare_changing_pointee(
+// CHECK: fir.alloca
+// CHECK: fir.declare
+// CHECK: fir.load
+func.func @declare_changing_pointee() {
+ %alloca = fir.alloca i32
+ %d = fir.declare %alloca {uniq_name = "x"} : (!fir.ref<i32>) -> !fir.ref<f32>
+ %l = fir.load %d : !fir.ref<f32>
+ "test.use"(%l) : (f32) -> ()
+ return
+}
+
+// -----
+
+// A memory space lets the cast relocate the storage, so it is not an alias.
+
+// CHECK-LABEL: func.func @memref_memory_space_not_promoted(
+// CHECK: memref.alloca
+// CHECK: fir.load
+func.func @memref_memory_space_not_promoted(%arg: i32) {
+ %alloca = memref.alloca() : memref<i32, 1>
+ %c = fir.convert %alloca : (memref<i32, 1>) -> !fir.ref<i32>
+ fir.store %arg to %c : !fir.ref<i32>
+ %l = fir.load %c : !fir.ref<i32>
+ "test.use"(%l) : (i32) -> ()
+ return
+}
+
+// -----
+
+// The address escapes through the cast, so the storage has to be kept.
+
+// CHECK-LABEL: func.func @escape_through_cast(
+// CHECK: memref.alloca
+// CHECK: fir.call @escapes
+// CHECK: fir.load
+func.func @escape_through_cast(%arg: i32) {
+ %alloca = memref.alloca() : memref<i32>
+ %c = fir.convert %alloca : (memref<i32>) -> !fir.ref<i32>
+ fir.store %arg to %c : !fir.ref<i32>
+ fir.call @escapes(%c) : (!fir.ref<i32>) -> ()
+ %l = fir.load %c : !fir.ref<i32>
+ "test.use"(%l) : (i32) -> ()
+ return
+}
+
+// -----
+
+// A cast may add volatility, which the default verifier accepts. Promotion
+// would elide the accesses that volatility asks to be kept.
+
+// CHECK-LABEL: func.func @volatile_convert_not_promoted(
+// CHECK: memref.alloca
+// CHECK: fir.store
+// CHECK: fir.load
+func.func @volatile_convert_not_promoted(%arg: i32) {
+ %alloca = memref.alloca() : memref<i32>
+ %c = fir.convert %alloca : (memref<i32>) -> !fir.ref<i32, volatile>
+ fir.store %arg to %c : !fir.ref<i32, volatile>
+ %l = fir.load %c : !fir.ref<i32, volatile>
+ "test.use"(%l) : (i32) -> ()
+ return
+}
>From 9bd11b2717dff6ce86d00773b5645455f28903ec Mon Sep 17 00:00:00 2001
From: Vijay Kandiah <vkandiah at nvidia.com>
Date: Fri, 28 Aug 2026 13:03:58 -0700
Subject: [PATCH 2/3] [flang] Only alias integer-like slots in mem2reg
---
flang/lib/Optimizer/Dialect/FIROps.cpp | 28 ++++++++++++++----
flang/test/Fir/mem2reg.mlir | 40 ++++++++++++++++++++++++++
2 files changed, 63 insertions(+), 5 deletions(-)
diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp
index 02bcec4a520c9..c183f3042afab 100644
--- a/flang/lib/Optimizer/Dialect/FIROps.cpp
+++ b/flang/lib/Optimizer/Dialect/FIROps.cpp
@@ -2136,6 +2136,15 @@ llvm::LogicalResult fir::ConvertOp::verify() {
<< getValue().getType() << " / " << getType();
}
+/// Check whether a slot of this pointee may be exposed as an alias. Promoting a
+/// floating-point slot lets its value be folded or, when nothing reads it,
+/// deleted, either of which loses an IEEE exception that the running program
+/// can observe through ieee_get_flag. Hence, restrict to Integer and Logical
+/// types.
+static bool isAliasableSlotPointee(mlir::Type pointee) {
+ return mlir::isa<mlir::IntegerType, fir::LogicalType>(pointee);
+}
+
/// Pointee of a reference to a simple scalar, or null. Both fir.ref and rank-0
/// memref qualify, since the storage of a scalar is cast between those forms.
static mlir::Type getScalarSlotPointeeType(mlir::Type type) {
@@ -2151,8 +2160,7 @@ static mlir::Type getScalarSlotPointeeType(mlir::Type type) {
} else {
return {};
}
- if (!mlir::isa<mlir::IntegerType, mlir::FloatType, mlir::ComplexType,
- fir::LogicalType>(eleTy))
+ if (!isAliasableSlotPointee(eleTy))
return {};
return eleTy;
}
@@ -6185,7 +6193,8 @@ void fir::DeclareOp::getPromotableSlotAliases(
const mlir::MemorySlot &parentSlot,
llvm::SmallVectorImpl<mlir::MemorySlot> &newMemorySlots) {
// fir.declare only attaches source information; the storage is the same.
- if (mlir::Type pointee = getPromotablePointeeOfDeclare(*this))
+ mlir::Type pointee = getPromotablePointeeOfDeclare(*this);
+ if (pointee && isAliasableSlotPointee(pointee))
newMemorySlots.push_back({getResult(), pointee});
}
@@ -6193,10 +6202,19 @@ bool fir::DeclareOp::canUsesBeRemoved(
const mlir::SmallPtrSetImpl<mlir::OpOperand *> &blockingUses,
mlir::SmallVectorImpl<mlir::OpOperand *> &newBlockingUses,
const mlir::DataLayout &dataLayout) {
- if (!getPromotablePointeeOfDeclare(*this))
+ mlir::Type pointee = getPromotablePointeeOfDeclare(*this);
+ if (!pointee)
return false;
- for (mlir::OpOperand &use : getResult().getUses())
+ // Without an alias, mem2reg does not see writes made through this declare as
+ // definitions, so uses have to stay in one block for no block argument to be
+ // needed.
+ bool aliased = isAliasableSlotPointee(pointee);
+ mlir::Block *declBlock = getOperation()->getBlock();
+ for (mlir::OpOperand &use : getResult().getUses()) {
+ if (!aliased && use.getOwner()->getBlock() != declBlock)
+ return false;
newBlockingUses.push_back(&use);
+ }
return true;
}
diff --git a/flang/test/Fir/mem2reg.mlir b/flang/test/Fir/mem2reg.mlir
index 8dee16102363c..a71d9bbeca5c5 100644
--- a/flang/test/Fir/mem2reg.mlir
+++ b/flang/test/Fir/mem2reg.mlir
@@ -410,6 +410,46 @@ func.func @declare_changing_pointee() {
// -----
+// A floating-point slot is not aliased, so it is promoted only when all its
+// uses are in one block, where no block argument is needed.
+
+// CHECK-LABEL: func.func @fp_declare_single_block(
+// CHECK-SAME: %[[ARG0:.*]]: f32) -> f32 {
+// CHECK-NOT: fir.alloca
+// CHECK: fir.declare_value %[[ARG0]]
+// CHECK: return %[[ARG0]] : f32
+func.func @fp_declare_single_block(%arg: f32) -> f32 {
+ %alloca = fir.alloca f32
+ %d = fir.declare %alloca {uniq_name = "_QFEx"} : (!fir.ref<f32>) -> !fir.ref<f32>
+ fir.store %arg to %d : !fir.ref<f32>
+ %v = fir.load %d : !fir.ref<f32>
+ return %v : f32
+}
+
+// -----
+
+// Without an alias, mem2reg does not see the write in the other block, so a
+// floating-point slot used across blocks stays in memory.
+
+// CHECK-LABEL: func.func @fp_declare_multi_block(
+// CHECK: fir.alloca f32
+// CHECK: fir.declare
+// CHECK: fir.load
+func.func @fp_declare_multi_block(%arg: f32, %cdt: i1) -> f32 {
+ %alloca = fir.alloca f32
+ %d = fir.declare %alloca {uniq_name = "_QFEx"} : (!fir.ref<f32>) -> !fir.ref<f32>
+ fir.store %arg to %d : !fir.ref<f32>
+ llvm.cond_br %cdt, ^bb1, ^bb2
+^bb1:
+ fir.store %arg to %d : !fir.ref<f32>
+ llvm.br ^bb2
+^bb2:
+ %v = fir.load %d : !fir.ref<f32>
+ return %v : f32
+}
+
+// -----
+
// A memory space lets the cast relocate the storage, so it is not an alias.
// CHECK-LABEL: func.func @memref_memory_space_not_promoted(
>From 36715b78d23f793153f5ced7365d52c326ca1b52 Mon Sep 17 00:00:00 2001
From: Vijay Kandiah <vkandiah at nvidia.com>
Date: Mon, 31 Aug 2026 08:52:22 -0700
Subject: [PATCH 3/3] [flang] Use the view interface and dominance for mem2reg
aliasing
---
flang/lib/Optimizer/Dialect/FIROps.cpp | 75 +++++++++++++++-----------
flang/test/Fir/mem2reg.mlir | 56 ++++++++++++++++++-
2 files changed, 98 insertions(+), 33 deletions(-)
diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp
index c183f3042afab..9412582c54ee7 100644
--- a/flang/lib/Optimizer/Dialect/FIROps.cpp
+++ b/flang/lib/Optimizer/Dialect/FIROps.cpp
@@ -28,6 +28,7 @@
#include "mlir/IR/BuiltinAttributes.h"
#include "mlir/IR/BuiltinOps.h"
#include "mlir/IR/Diagnostics.h"
+#include "mlir/IR/Dominance.h"
#include "mlir/IR/Matchers.h"
#include "mlir/IR/OpDefinition.h"
#include "mlir/IR/PatternMatch.h"
@@ -2165,22 +2166,30 @@ static mlir::Type getScalarSlotPointeeType(mlir::Type type) {
return eleTy;
}
-/// The value a cast or declare aliases, or null if `value` is not one of those.
+/// Returns the source that 'value' is a view of at offset zero, or null. A
+/// displaced view, such as an array element, addresses a different location and
+/// so is a different slot.
static mlir::Value getAliasedSlotPointer(mlir::Value value) {
- mlir::Operation *def = value.getDefiningOp();
- if (auto convert = mlir::dyn_cast_or_null<fir::ConvertOp>(def))
- return convert.getValue();
- if (auto declare = mlir::dyn_cast_or_null<fir::DeclareOp>(def))
- return declare.getMemref();
- return {};
+ auto result = mlir::dyn_cast<mlir::OpResult>(value);
+ if (!result)
+ return {};
+ auto view =
+ mlir::dyn_cast<fir::FortranObjectViewOpInterface>(result.getOwner());
+ if (!view)
+ return {};
+ std::optional<std::int64_t> offset = view.getViewOffset(result);
+ if (!offset || *offset != 0)
+ return {};
+ return view.getViewSource(result);
}
-/// Checks whether every read of the slot reached through `pointer` has a write
-/// to it earlier in the same block. Otherwise promotion replaces the read with
-/// the allocation's default value, which is fir.undefined for fir.alloca but
-/// poison for a memref allocation, so promoting through a cast would change
-/// what reading an uninitialized variable does.
-static bool everyReadFollowsAWrite(mlir::Value pointer, mlir::Type pointee) {
+/// Checks whether a write to the slot reached through `pointer` dominates every
+/// read of it. Otherwise promotion replaces a read with the allocation's
+/// default value, which is fir.undefined for fir.alloca but poison for a memref
+/// allocation, so promoting through a view would change what reading an
+/// uninitialized variable does.
+static bool everyReadIsDominatedByAWrite(mlir::Value pointer,
+ mlir::Type pointee) {
// Start at the root of the chain, to see writes through a sibling alias.
while (mlir::Value aliased = getAliasedSlotPointer(pointer)) {
if (getScalarSlotPointeeType(aliased.getType()) != pointee)
@@ -2189,37 +2198,39 @@ static bool everyReadFollowsAWrite(mlir::Value pointer, mlir::Type pointee) {
}
llvm::SmallVector<mlir::Operation *> reads;
- llvm::DenseMap<mlir::Block *, mlir::Operation *> firstWrite;
+ llvm::SmallVector<mlir::Operation *> writes;
llvm::SmallVector<mlir::Value> worklist{pointer};
llvm::SmallPtrSet<mlir::Value, 8> visited{pointer};
while (!worklist.empty()) {
- for (mlir::OpOperand &use : worklist.pop_back_val().getUses()) {
+ mlir::Value slotPointer = worklist.pop_back_val();
+ for (mlir::OpOperand &use : slotPointer.getUses()) {
mlir::Operation *user = use.getOwner();
- if (mlir::isa<fir::ConvertOp, fir::DeclareOp>(user)) {
- mlir::Value alias = user->getResult(0);
+ // A view of the pointer is another handle on the same storage, so its
+ // own uses have to be walked as well.
+ for (mlir::OpResult result : user->getOpResults()) {
+ if (getAliasedSlotPointer(result) != slotPointer)
+ continue;
// A different pointee is another slot, which blocks promotion itself.
- if (getScalarSlotPointeeType(alias.getType()) == pointee &&
- visited.insert(alias).second)
- worklist.push_back(alias);
- continue;
+ if (getScalarSlotPointeeType(result.getType()) == pointee &&
+ visited.insert(result).second)
+ worklist.push_back(result);
}
// A user that is not a promotable access blocks promotion itself.
if (auto memOp = mlir::dyn_cast<mlir::PromotableMemOpInterface>(user)) {
mlir::MemorySlot slot{use.get(), pointee};
- if (memOp.storesTo(slot)) {
- mlir::Operation *&earliest = firstWrite[user->getBlock()];
- if (!earliest || user->isBeforeInBlock(earliest))
- earliest = user;
- } else if (memOp.loadsFrom(slot)) {
+ if (memOp.storesTo(slot))
+ writes.push_back(user);
+ else if (memOp.loadsFrom(slot))
reads.push_back(user);
- }
}
}
}
- return llvm::all_of(reads, [&firstWrite](mlir::Operation *read) {
- auto write = firstWrite.find(read->getBlock());
- return write != firstWrite.end() && write->second->isBeforeInBlock(read);
+ mlir::DominanceInfo dominance;
+ return llvm::all_of(reads, [&](mlir::Operation *read) {
+ return llvm::any_of(writes, [&](mlir::Operation *write) {
+ return dominance.properlyDominates(write, read);
+ });
});
}
@@ -2237,8 +2248,8 @@ static mlir::Type getPointeePreservedByCast(fir::ConvertOp op) {
mlir::Type to = getScalarSlotPointeeType(toTy);
if (!from || from != to)
return {};
- // A read that no write reaches must keep its access.
- if (!everyReadFollowsAWrite(op.getValue(), to))
+ // A read that no write dominates must keep its access.
+ if (!everyReadIsDominatedByAWrite(op.getValue(), to))
return {};
return to;
}
diff --git a/flang/test/Fir/mem2reg.mlir b/flang/test/Fir/mem2reg.mlir
index a71d9bbeca5c5..56103071bd870 100644
--- a/flang/test/Fir/mem2reg.mlir
+++ b/flang/test/Fir/mem2reg.mlir
@@ -352,7 +352,7 @@ func.func @read_only_slot_not_promoted() {
// -----
-// Written on one path only, so no write reaches the read in the entry block.
+// Written inside a conditional region, so no write dominates the read.
// CHECK-LABEL: func.func @partially_written_slot_not_promoted(
// CHECK-NOT: ub.poison
@@ -499,3 +499,57 @@ func.func @volatile_convert_not_promoted(%arg: i32) {
"test.use"(%l) : (i32) -> ()
return
}
+
+// -----
+
+// The write dominates the read from another block, so the read cannot observe
+// the uninitialized slot and no poison is needed.
+
+// CHECK-LABEL: func.func @write_dominating_read_in_another_block(
+// CHECK-SAME: %[[ARG:.*]]: i32
+// CHECK-NOT: memref.alloca
+// CHECK-NOT: ub.poison
+// CHECK: "test.use"(%[[ARG]])
+func.func @write_dominating_read_in_another_block(%arg: i32) {
+ %alloca = memref.alloca() {bindc_name = "x"} : memref<i32>
+ %r = fir.convert %alloca : (memref<i32>) -> !fir.ref<i32>
+ %d = fir.declare %r {uniq_name = "_QFEx"} : (!fir.ref<i32>) -> !fir.ref<i32>
+ %m = fir.convert %d : (!fir.ref<i32>) -> memref<i32>
+ memref.store %arg, %m[] : memref<i32>
+ cf.br ^bb1
+^bb1:
+ %l = memref.load %m[] : memref<i32>
+ "test.use"(%l) : (i32) -> ()
+ return
+}
+
+// -----
+
+// Initialized before a loop and updated inside it. The initializing write
+// dominates the in-loop read, so the slot becomes a block argument.
+
+// CHECK-LABEL: func.func @write_before_loop_updated_in_loop(
+// CHECK-SAME: %[[ARG:[^:]*]]: i32, %[[N:[^:]*]]: i32
+// CHECK-NOT: memref.alloca
+// CHECK-NOT: ub.poison
+// CHECK: cf.br ^bb1(%[[ARG]] : i32)
+// CHECK: ^bb1(%[[PHI:.*]]: i32)
+// CHECK: arith.addi %[[PHI]], %[[ARG]]
+func.func @write_before_loop_updated_in_loop(%arg: i32, %n: i32) {
+ %alloca = memref.alloca() {bindc_name = "x"} : memref<i32>
+ %r = fir.convert %alloca : (memref<i32>) -> !fir.ref<i32>
+ %d = fir.declare %r {uniq_name = "_QFEx"} : (!fir.ref<i32>) -> !fir.ref<i32>
+ %m = fir.convert %d : (!fir.ref<i32>) -> memref<i32>
+ memref.store %arg, %m[] : memref<i32>
+ cf.br ^bb1
+^bb1:
+ %l = memref.load %m[] : memref<i32>
+ %next = arith.addi %l, %arg : i32
+ memref.store %next, %m[] : memref<i32>
+ %c = arith.cmpi slt, %next, %n : i32
+ cf.cond_br %c, ^bb1, ^bb2
+^bb2:
+ %f = memref.load %m[] : memref<i32>
+ "test.use"(%f) : (i32) -> ()
+ return
+}
More information about the flang-commits
mailing list