Skip to content

Commit 9d7177a

Browse files
Renaud-Kjdenny-ornl
Renaud-K
and
jdenny-ornl
authored
[flang][NFCI] Stop tracking memory source after a load in a more explicit manner. (#126156)
Typically, we do not track memory sources after a load because of the dynamic nature of the load and the fact that the alias analysis is a simple static analysis. However, the code is written in a way that makes it seem like we are continuing to track memory but in reality we are only doing so when we know that the tracked memory is a leaf and therefore when there will only be one more iteration through the switch statement. In other words, we are iterating one more time, to gather data about a box, anticipating that this will be the last time. This is a hack that helped avoid cut-and-paste from other case statements but gives the wrong impression about the intention of the code and makes it confusing. To make it clear that there is no more tracking, we gather all the necessary data from the memref of the load, in the case statement for the load, and exit the loop. I am also limiting this data gathering for the case when we load a box reference while we were actually following data, as tests have shows, is the only case when we need it for. Other cases will be handled conservatively, but this can change in the future, on a case-by-case basis. --------- Co-authored-by: Joel E. Denny <jdenny.ornl@gmail.com>
1 parent 71e623d commit 9d7177a

File tree

3 files changed

+122
-14
lines changed

3 files changed

+122
-14
lines changed

‎flang/lib/Optimizer/Analysis/AliasAnalysis.cpp

+35-11
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ static bool hasGlobalOpTargetAttr(mlir::Value v, fir::AddrOfOp op) {
5151
v, fir::GlobalOp::getTargetAttrName(globalOpName));
5252
}
5353

54-
mlir::Value getOriginalDef(mlir::Value v) {
54+
static mlir::Value getOriginalDef(mlir::Value v) {
5555
mlir::Operation *defOp;
5656
bool breakFromLoop = false;
5757
while (!breakFromLoop && (defOp = v.getDefiningOp())) {
@@ -578,16 +578,6 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
578578
breakFromLoop = true;
579579
})
580580
.Case<fir::LoadOp>([&](auto op) {
581-
// If the load is from a leaf source, return the leaf. Do not track
582-
// through indirections otherwise.
583-
// TODO: Add support to fir.alloca and fir.allocmem
584-
auto def = getOriginalDef(op.getMemref());
585-
if (isDummyArgument(def) ||
586-
def.template getDefiningOp<fir::AddrOfOp>()) {
587-
v = def;
588-
defOp = v.getDefiningOp();
589-
return;
590-
}
591581
// If load is inside target and it points to mapped item,
592582
// continue tracking.
593583
Operation *loadMemrefOp = op.getMemref().getDefiningOp();
@@ -600,6 +590,40 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
600590
defOp = v.getDefiningOp();
601591
return;
602592
}
593+
594+
// If we are loading a box reference, but following the data,
595+
// we gather the attributes of the box to populate the source
596+
// and stop tracking.
597+
if (auto boxTy = mlir::dyn_cast<fir::BaseBoxType>(ty);
598+
boxTy && followingData) {
599+
600+
if (mlir::isa<fir::PointerType>(boxTy.getEleTy()))
601+
attributes.set(Attribute::Pointer);
602+
603+
auto def = getOriginalDef(op.getMemref());
604+
if (auto addrOfOp = def.template getDefiningOp<fir::AddrOfOp>()) {
605+
global = addrOfOp.getSymbol();
606+
607+
if (hasGlobalOpTargetAttr(def, addrOfOp))
608+
attributes.set(Attribute::Target);
609+
610+
type = SourceKind::Global;
611+
}
612+
613+
// TODO: Add support to fir.alloca and fir.allocmem
614+
// if (auto allocOp = def.template getDefiningOp<fir::AllocaOp>()) {
615+
// ...
616+
// }
617+
618+
if (isDummyArgument(def)) {
619+
defOp = nullptr;
620+
v = def;
621+
}
622+
623+
breakFromLoop = true;
624+
return;
625+
}
626+
603627
// No further tracking for addresses loaded from memory for now.
604628
type = SourceKind::Indirect;
605629
breakFromLoop = true;

‎flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir

+5-3
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,15 @@
4747
// CHECK-DAG: arg2.load#0 <-> arg2.addr#0: MustAlias
4848
// CHECK-DAG: boxp1.addr#0 <-> arg2.addr#0: MayAlias
4949

50-
// TODO: Can the address in a pointer alias the address of a pointer, even when the
50+
// TODO: Can the address in a pointer alias the address of a pointer, when the
5151
// pointer has no box. Should this be NoAlias?
52-
// T3: CHECK-DAG: p1.addr#0 <-> p1.tgt#0: MayAlias
52+
// T3 from <https://github.com/llvm/llvm-project/pull/117785#discussion_r1924348480>.
53+
// CHECK-DAG: p1.addr#0 <-> p1.tgt#0: MayAlias
5354

5455
// The addresses stored in two different pointers can alias, even if one has no
5556
// box. In this program, they happen to be the same address.
56-
// T4: CHECK-DAG: p1.tgt#0 <-> boxp1.addr#0: MayAlias
57+
// T4:
58+
// CHECK-DAG: p1.tgt#0 <-> boxp1.addr#0: MayAlias
5759

5860
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"} {
5961

‎flang/test/Analysis/AliasAnalysis/alias-analysis-target.fir

+82
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
// RUN: fir-opt %s -pass-pipeline='builtin.module(func.func(test-fir-alias-analysis))' 2>&1 | FileCheck %s
2+
3+
// The test was obtained from
4+
// bbc test.f90 -emit-fir
5+
// module mod
6+
// real, pointer :: p0
7+
// real, allocatable :: alloc
8+
// real, allocatable, target :: t_alloc
9+
// real, target :: t
10+
// real :: v
11+
// end module
12+
//
13+
// subroutine test(n)
14+
// use mod
15+
// integer :: n
16+
// real r1
17+
// p0 => t_alloc
18+
// v = alloc
19+
// r1 = p0
20+
// end subroutine test
21+
22+
// Checking that aliasing can only happen with an entity with the target attribute
23+
//
24+
// CHECK-DAG: r1#0 <-> t_alloc#0: NoAlias
25+
// CHECK-DAG: r1#0 <-> alloc#0: NoAlias
26+
// CHECK-DAG: t_alloc#0 <-> alloc#0: NoAlias
27+
// CHECK-DAG: r1#0 <-> p0.ptr#0: NoAlias
28+
// CHECK-DAG: t_alloc#0 <-> p0.ptr#0: MayAlias
29+
// CHECK-DAG: alloc#0 <-> p0.ptr#0: NoAlias
30+
31+
fir.global @_QMmodEalloc : !fir.box<!fir.heap<f32>> {
32+
%0 = fir.zero_bits !fir.heap<f32>
33+
%1 = fir.embox %0 : (!fir.heap<f32>) -> !fir.box<!fir.heap<f32>>
34+
fir.has_value %1 : !fir.box<!fir.heap<f32>>
35+
}
36+
fir.global @_QMmodEp0 : !fir.box<!fir.ptr<f32>> {
37+
%0 = fir.zero_bits !fir.ptr<f32>
38+
%1 = fir.embox %0 : (!fir.ptr<f32>) -> !fir.box<!fir.ptr<f32>>
39+
fir.has_value %1 : !fir.box<!fir.ptr<f32>>
40+
}
41+
fir.global @_QMmodEt target : f32 {
42+
%0 = fir.zero_bits f32
43+
fir.has_value %0 : f32
44+
}
45+
fir.global @_QMmodEt_alloc target : !fir.box<!fir.heap<f32>> {
46+
%0 = fir.zero_bits !fir.heap<f32>
47+
%1 = fir.embox %0 : (!fir.heap<f32>) -> !fir.box<!fir.heap<f32>>
48+
fir.has_value %1 : !fir.box<!fir.heap<f32>>
49+
}
50+
fir.global @_QMmodEv : f32 {
51+
%0 = fir.zero_bits f32
52+
fir.has_value %0 : f32
53+
}
54+
func.func @_QPtest(%arg0: !fir.ref<i32> {fir.bindc_name = "n"}) {
55+
%0 = fir.dummy_scope : !fir.dscope
56+
%1 = fir.address_of(@_QMmodEalloc) : !fir.ref<!fir.box<!fir.heap<f32>>>
57+
%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>>>
58+
%3 = fir.declare %arg0 dummy_scope %0 {uniq_name = "_QFtestEn"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
59+
%4 = fir.address_of(@_QMmodEp0) : !fir.ref<!fir.box<!fir.ptr<f32>>>
60+
%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>>>
61+
%6 = fir.alloca f32 {bindc_name = "r1", uniq_name = "_QFtestEr1"}
62+
%7 = fir.declare %6 {test.ptr="r1", uniq_name = "_QFtestEr1"} : (!fir.ref<f32>) -> !fir.ref<f32>
63+
%8 = fir.address_of(@_QMmodEt) : !fir.ref<f32>
64+
%9 = fir.declare %8 {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QMmodEt"} : (!fir.ref<f32>) -> !fir.ref<f32>
65+
%10 = fir.address_of(@_QMmodEt_alloc) : !fir.ref<!fir.box<!fir.heap<f32>>>
66+
%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>>>
67+
%12 = fir.address_of(@_QMmodEv) : !fir.ref<f32>
68+
%13 = fir.declare %12 {uniq_name = "_QMmodEv"} : (!fir.ref<f32>) -> !fir.ref<f32>
69+
%14 = fir.load %11 : !fir.ref<!fir.box<!fir.heap<f32>>>
70+
%15 = fir.box_addr %14 {test.ptr="t_alloc"}: (!fir.box<!fir.heap<f32>>) -> !fir.heap<f32>
71+
%16 = fir.embox %15 : (!fir.heap<f32>) -> !fir.box<!fir.ptr<f32>>
72+
fir.store %16 to %5 : !fir.ref<!fir.box<!fir.ptr<f32>>>
73+
%17 = fir.load %2 : !fir.ref<!fir.box<!fir.heap<f32>>>
74+
%18 = fir.box_addr %17 {test.ptr="alloc"} : (!fir.box<!fir.heap<f32>>) -> !fir.heap<f32>
75+
%19 = fir.load %18 : !fir.heap<f32>
76+
fir.store %19 to %13 : !fir.ref<f32>
77+
%20 = fir.load %5 : !fir.ref<!fir.box<!fir.ptr<f32>>>
78+
%21 = fir.box_addr %20 {test.ptr="p0.ptr"} : (!fir.box<!fir.ptr<f32>>) -> !fir.ptr<f32>
79+
%22 = fir.load %21 : !fir.ptr<f32>
80+
fir.store %22 to %7 : !fir.ref<f32>
81+
return
82+
}

0 commit comments

Comments
 (0)