[flang-commits] [flang] [flang][NFCI] Stop tracking memory source after a load in a more explicit manner. (PR #126156)
Renaud Kauffmann via flang-commits
flang-commits at lists.llvm.org
Fri Feb 7 11:30:53 PST 2025
https://github.com/Renaud-K updated https://github.com/llvm/llvm-project/pull/126156
>From 7299f21b20cedf0fa902cdc8081039a367d207a2 Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Thu, 6 Feb 2025 16:10:57 -0800
Subject: [PATCH 1/5] Applying formatting
---
.../lib/Optimizer/Analysis/AliasAnalysis.cpp | 47 ++++++++++++++-----
.../AliasAnalysis/alias-analysis-2.fir | 6 ++-
2 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index 01f3a0326db216e..ac2de2b6202c37c 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -51,7 +51,7 @@ static bool hasGlobalOpTargetAttr(mlir::Value v, fir::AddrOfOp op) {
v, fir::GlobalOp::getTargetAttrName(globalOpName));
}
-mlir::Value getOriginalDef(mlir::Value v) {
+static mlir::Value getOriginalDef(mlir::Value v) {
mlir::Operation *defOp;
bool breakFromLoop = false;
while (!breakFromLoop && (defOp = v.getDefiningOp())) {
@@ -578,16 +578,6 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
breakFromLoop = true;
})
.Case<fir::LoadOp>([&](auto op) {
- // If the load is from a leaf source, return the leaf. Do not track
- // through indirections otherwise.
- // TODO: Add support to fir.alloca and fir.allocmem
- auto def = getOriginalDef(op.getMemref());
- if (isDummyArgument(def) ||
- def.template getDefiningOp<fir::AddrOfOp>()) {
- v = def;
- defOp = v.getDefiningOp();
- return;
- }
// If load is inside target and it points to mapped item,
// continue tracking.
Operation *loadMemrefOp = op.getMemref().getDefiningOp();
@@ -600,6 +590,41 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
defOp = v.getDefiningOp();
return;
}
+
+ // If we are loading a box reference, but following the data,
+ // we gather the attributes of the box to populate the source
+ // and stop tracking.
+ if (auto boxTy = mlir::dyn_cast<fir::BaseBoxType>(ty);
+ boxTy && followingData) {
+
+ if (mlir::isa<fir::PointerType>(boxTy.getEleTy())) {
+ attributes.set(Attribute::Pointer);
+ }
+
+ auto def = getOriginalDef(op.getMemref());
+ if (auto addrOfOp = def.template getDefiningOp<fir::AddrOfOp>()) {
+ global = addrOfOp.getSymbol();
+
+ if (hasGlobalOpTargetAttr(def, addrOfOp))
+ attributes.set(Attribute::Target);
+
+ type = SourceKind::Global;
+ }
+
+ // TODO: Add support to fir.alloca and fir.allocmem
+ // if (auto allocOp = def.template getDefiningOp<fir::AllocaOp>()) {
+ // ...
+ // }
+
+ if (isDummyArgument(def)) {
+ defOp = nullptr;
+ v = def;
+ }
+
+ breakFromLoop = true;
+ return;
+ }
+
// No further tracking for addresses loaded from memory for now.
type = SourceKind::Indirect;
breakFromLoop = true;
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
index ca97c5900281d64..3fbf29ab2eb2926 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
@@ -49,11 +49,13 @@
// TODO: Can the address in a pointer alias the address of a pointer, even when the
// pointer has no box. Should this be NoAlias?
-// T3: CHECK-DAG: p1.addr#0 <-> p1.tgt#0: MayAlias
+// T3:
+// CHECK-DAG: p1.addr#0 <-> p1.tgt#0: MayAlias
// The addresses stored in two different pointers can alias, even if one has no
// box. In this program, they happen to be the same address.
-// T4: CHECK-DAG: p1.tgt#0 <-> boxp1.addr#0: MayAlias
+// T4:
+// CHECK-DAG: p1.tgt#0 <-> boxp1.addr#0: MayAlias
func.func @_QFPtest(%arg0: !fir.ref<f32> {fir.bindc_name = "v1", fir.target}, %arg1: !fir.ref<f32> {fir.bindc_name = "v2", fir.target}, %arg2: !fir.ref<!fir.box<!fir.ptr<f32>>> ) attributes {test.ptr = "func"} {
>From 8d43bd499923cba34489f55cf72c28ebdfde07f2 Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Fri, 7 Feb 2025 08:55:39 -0800
Subject: [PATCH 2/5] Removing curly brackets
---
flang/lib/Optimizer/Analysis/AliasAnalysis.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index ac2de2b6202c37c..5827d1c3c529ed2 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -597,9 +597,8 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
if (auto boxTy = mlir::dyn_cast<fir::BaseBoxType>(ty);
boxTy && followingData) {
- if (mlir::isa<fir::PointerType>(boxTy.getEleTy())) {
+ if (mlir::isa<fir::PointerType>(boxTy.getEleTy()))
attributes.set(Attribute::Pointer);
- }
auto def = getOriginalDef(op.getMemref());
if (auto addrOfOp = def.template getDefiningOp<fir::AddrOfOp>()) {
>From 3139abc33651c6abf8bbdba4446e0cef4c0af9dc Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Fri, 7 Feb 2025 09:39:59 -0800
Subject: [PATCH 3/5] Adding a test that makes sure that we are reading the
target attribute on globals
---
.../AliasAnalysis/alias-analysis-target.fir | 82 +++++++++++++++++++
1 file changed, 82 insertions(+)
create mode 100644 flang/test/Analysis/AliasAnalysis/alias-analysis-target.fir
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-target.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-target.fir
new file mode 100644
index 000000000000000..8e88b508d56e372
--- /dev/null
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-target.fir
@@ -0,0 +1,82 @@
+// RUN: fir-opt %s -pass-pipeline='builtin.module(func.func(test-fir-alias-analysis))' 2>&1 | FileCheck %s
+
+// The test was obtained from
+// bbc test.f90 -emit-fir
+// module mod
+// real, pointer :: p0
+// real, allocatable :: alloc
+// real, allocatable, target :: t_alloc
+// real, target :: t
+// real :: v
+// end module
+//
+// subroutine test(n)
+// use mod
+// integer :: n
+// real r1
+// p0 => t_alloc
+// v = alloc
+// r1 = p0
+// end subroutine test
+
+// Checking that aliasing can only happen with an entity with the target attribute
+//
+// CHECK-DAG: r1#0 <-> t_alloc#0: NoAlias
+// CHECK-DAG: r1#0 <-> alloc#0: NoAlias
+// CHECK-DAG: t_alloc#0 <-> alloc#0: NoAlias
+// CHECK-DAG: r1#0 <-> p0.ptr#0: NoAlias
+// CHECK-DAG: t_alloc#0 <-> p0.ptr#0: MayAlias
+// CHECK-DAG: alloc#0 <-> p0.ptr#0: NoAlias
+
+fir.global @_QMmodEalloc : !fir.box<!fir.heap<f32>> {
+ %0 = fir.zero_bits !fir.heap<f32>
+ %1 = fir.embox %0 : (!fir.heap<f32>) -> !fir.box<!fir.heap<f32>>
+ fir.has_value %1 : !fir.box<!fir.heap<f32>>
+}
+fir.global @_QMmodEp0 : !fir.box<!fir.ptr<f32>> {
+ %0 = fir.zero_bits !fir.ptr<f32>
+ %1 = fir.embox %0 : (!fir.ptr<f32>) -> !fir.box<!fir.ptr<f32>>
+ fir.has_value %1 : !fir.box<!fir.ptr<f32>>
+}
+fir.global @_QMmodEt target : f32 {
+ %0 = fir.zero_bits f32
+ fir.has_value %0 : f32
+}
+fir.global @_QMmodEt_alloc target : !fir.box<!fir.heap<f32>> {
+ %0 = fir.zero_bits !fir.heap<f32>
+ %1 = fir.embox %0 : (!fir.heap<f32>) -> !fir.box<!fir.heap<f32>>
+ fir.has_value %1 : !fir.box<!fir.heap<f32>>
+}
+fir.global @_QMmodEv : f32 {
+ %0 = fir.zero_bits f32
+ fir.has_value %0 : f32
+}
+func.func @_QPtest(%arg0: !fir.ref<i32> {fir.bindc_name = "n"}) {
+ %0 = fir.dummy_scope : !fir.dscope
+ %1 = fir.address_of(@_QMmodEalloc) : !fir.ref<!fir.box<!fir.heap<f32>>>
+ %2 = fir.declare %1 {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "_QMmodEalloc"} : (!fir.ref<!fir.box<!fir.heap<f32>>>) -> !fir.ref<!fir.box<!fir.heap<f32>>>
+ %3 = fir.declare %arg0 dummy_scope %0 {uniq_name = "_QFtestEn"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
+ %4 = fir.address_of(@_QMmodEp0) : !fir.ref<!fir.box<!fir.ptr<f32>>>
+ %5 = fir.declare %4 {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QMmodEp0"} : (!fir.ref<!fir.box<!fir.ptr<f32>>>) -> !fir.ref<!fir.box<!fir.ptr<f32>>>
+ %6 = fir.alloca f32 {bindc_name = "r1", uniq_name = "_QFtestEr1"}
+ %7 = fir.declare %6 {test.ptr="r1", uniq_name = "_QFtestEr1"} : (!fir.ref<f32>) -> !fir.ref<f32>
+ %8 = fir.address_of(@_QMmodEt) : !fir.ref<f32>
+ %9 = fir.declare %8 {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QMmodEt"} : (!fir.ref<f32>) -> !fir.ref<f32>
+ %10 = fir.address_of(@_QMmodEt_alloc) : !fir.ref<!fir.box<!fir.heap<f32>>>
+ %11 = fir.declare %10 {fortran_attrs = #fir.var_attrs<allocatable, target>, uniq_name = "_QMmodEt_alloc"} : (!fir.ref<!fir.box<!fir.heap<f32>>>) -> !fir.ref<!fir.box<!fir.heap<f32>>>
+ %12 = fir.address_of(@_QMmodEv) : !fir.ref<f32>
+ %13 = fir.declare %12 {uniq_name = "_QMmodEv"} : (!fir.ref<f32>) -> !fir.ref<f32>
+ %14 = fir.load %11 : !fir.ref<!fir.box<!fir.heap<f32>>>
+ %15 = fir.box_addr %14 {test.ptr="t_alloc"}: (!fir.box<!fir.heap<f32>>) -> !fir.heap<f32>
+ %16 = fir.embox %15 : (!fir.heap<f32>) -> !fir.box<!fir.ptr<f32>>
+ fir.store %16 to %5 : !fir.ref<!fir.box<!fir.ptr<f32>>>
+ %17 = fir.load %2 : !fir.ref<!fir.box<!fir.heap<f32>>>
+ %18 = fir.box_addr %17 {test.ptr="alloc"} : (!fir.box<!fir.heap<f32>>) -> !fir.heap<f32>
+ %19 = fir.load %18 : !fir.heap<f32>
+ fir.store %19 to %13 : !fir.ref<f32>
+ %20 = fir.load %5 : !fir.ref<!fir.box<!fir.ptr<f32>>>
+ %21 = fir.box_addr %20 {test.ptr="p0.ptr"} : (!fir.box<!fir.ptr<f32>>) -> !fir.ptr<f32>
+ %22 = fir.load %21 : !fir.ptr<f32>
+ fir.store %22 to %7 : !fir.ref<f32>
+ return
+}
>From ff140e7e56a5a2d901d8ed4adbbbede63b578b28 Mon Sep 17 00:00:00 2001
From: Renaud Kauffmann <rkauffmann at nvidia.com>
Date: Fri, 7 Feb 2025 11:21:16 -0800
Subject: [PATCH 4/5] Update
flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
Point to where T3 is discussed.
Co-authored-by: Joel E. Denny <jdenny.ornl at gmail.com>
---
flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
index 3fbf29ab2eb2926..c4af76c5cc494f4 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
@@ -49,7 +49,7 @@
// TODO: Can the address in a pointer alias the address of a pointer, even when the
// pointer has no box. Should this be NoAlias?
-// T3:
+// T3 from <https://github.com/llvm/llvm-project/pull/117785#discussion_r1924348480>.
// CHECK-DAG: p1.addr#0 <-> p1.tgt#0: MayAlias
// The addresses stored in two different pointers can alias, even if one has no
>From 7f7b43c4beed52ea38f41307e4e50297e347d9f3 Mon Sep 17 00:00:00 2001
From: Renaud Kauffmann <rkauffmann at nvidia.com>
Date: Fri, 7 Feb 2025 11:30:42 -0800
Subject: [PATCH 5/5] Update
flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
Slight comment change.
Co-authored-by: Joel E. Denny <jdenny.ornl at gmail.com>
---
flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
index c4af76c5cc494f4..24cfaf6ed7ecc91 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
@@ -47,7 +47,7 @@
// CHECK-DAG: arg2.load#0 <-> arg2.addr#0: MustAlias
// CHECK-DAG: boxp1.addr#0 <-> arg2.addr#0: MayAlias
-// TODO: Can the address in a pointer alias the address of a pointer, even when the
+// TODO: Can the address in a pointer alias the address of a pointer, when the
// pointer has no box. Should this be NoAlias?
// T3 from <https://github.com/llvm/llvm-project/pull/117785#discussion_r1924348480>.
// CHECK-DAG: p1.addr#0 <-> p1.tgt#0: MayAlias
More information about the flang-commits
mailing list