Skip to content

Commit cea1244

Browse files
Renaud-KRenaud-K
committed
Applying review feedback and additional clean-ups
1 parent cf10093 commit cea1244

File tree

3 files changed

+126
-44
lines changed

3 files changed

+126
-44
lines changed

‎flang/include/flang/Optimizer/Analysis/AliasAnalysis.h‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,9 @@ struct AliasAnalysis {
3636
/// Represents memory allocated outside of a function
3737
/// and passed to the function via host association tuple.
3838
HostAssoc,
39-
/// Represents direct memory access whose source cannot be further
40-
/// determined
39+
/// Represents memory allocated by unknown means and
40+
/// with the memory address defined by a memory reading
41+
/// operation (e.g. fir::LoadOp).
4142
Indirect,
4243
/// Starting point to the analysis whereby nothing is known about
4344
/// the source
@@ -46,6 +47,56 @@ struct AliasAnalysis {
4647
/// Attributes of the memory source object.
4748
ENUM_CLASS(Attribute, Target, Pointer, IntentIn);
4849

50+
// See
51+
// https://discourse.llvm.org/t/rfc-distinguish-between-data-and-non-data-in-fir-alias-analysis/78759/1
52+
//
53+
// It is possible, while following the source of a memory reference through
54+
// the use-def chain, to arrive at the same origin, even though the starting
55+
// points were known to not alias.
56+
//
57+
// Example:
58+
// clang-format off
59+
// fir.global @_QMtopEa : !fir.box<!fir.ptr<!fir.array<?xf32>>>
60+
//
61+
// func.func @_QPtest() {
62+
// %c1 = arith.constant 1 : index
63+
// %cst = arith.constant 1.000000e+00 : f32
64+
// %0 = fir.address_of(@_QMtopEa) : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
65+
// %1 = fir.declare %0 {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QMtopEa"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>) -> !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
66+
// %2 = fir.load %1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
67+
// ...
68+
// %5 = fir.array_coor %2 %c1 : (!fir.box<!fir.ptr<!fir.array<?xf32>>>, !fir.shift<1>, index) -> !fir.ref<f32>
69+
// fir.store %cst to %5 : !fir.ref<f32>
70+
// return
71+
// }
72+
//
73+
// With high level operations, such as fir.array_coor, it is possible to
74+
// reach into the data wrapped by the box (the descriptor) therefore when
75+
// asking about the memory source of the %5, we are really asking about the
76+
// source of the data of box %2.
77+
//
78+
// When asking about the source of %0 which is the address of the box, we
79+
// reach the same source as in the first case: the global @_QMtopEa. Yet one
80+
// source refers to the data while the other refers to the address of the box
81+
// itself.
82+
//
83+
// To distinguish between the two, the isData flag has been added, whereby
84+
// data is defined as any memory reference that is not a box reference.
85+
// Additionally, because it is relied on in HLFIR lowering, we allow querying
86+
// on a box SSA value, which is interpreted as querying on its data.
87+
//
88+
// So in the above example, !fir.ref<f32> and !fir.box<!fir.ptr<!fir.array<?xf32>>> is data,
89+
// while !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>> is not data.
90+
91+
// This generally applies to function arguments. In the example below, %arg0
92+
// is data, %arg1 is not data but a load of %arg1 is.
93+
//
94+
// func.func @_QFPtest2(%arg0: !fir.ref<f32>, %arg1: !fir.ref<!fir.box<!fir.ptr<f32>>> ) {
95+
// %0 = fir.load %arg1 : !fir.ref<!fir.box<!fir.ptr<f32>>>
96+
// ... }
97+
//
98+
// clang-format on
99+
49100
struct Source {
50101
using SourceUnion = llvm::PointerUnion<mlir::SymbolRefAttr, mlir::Value>;
51102
using Attributes = Fortran::common::EnumSet<Attribute, Attribute_enumSize>;

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

Lines changed: 22 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,11 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
108108
auto lhsSrc = getSource(lhs);
109109
auto rhsSrc = getSource(rhs);
110110
bool approximateSource = lhsSrc.approximateSource || rhsSrc.approximateSource;
111-
LLVM_DEBUG(llvm::dbgs() << "AliasAnalysis::alias\n";
111+
LLVM_DEBUG(llvm::dbgs() << "\n"; llvm::dbgs() << "AliasAnalysis::alias\n";
112112
llvm::dbgs() << " lhs: " << lhs << "\n";
113113
llvm::dbgs() << " lhsSrc: " << lhsSrc << "\n";
114114
llvm::dbgs() << " rhs: " << rhs << "\n";
115-
llvm::dbgs() << " rhsSrc: " << rhsSrc << "\n";
116-
llvm::dbgs() << "\n";);
115+
llvm::dbgs() << " rhsSrc: " << rhsSrc << "\n";);
117116

118117
// Indirect case currently not handled. Conservatively assume
119118
// it aliases with everything
@@ -122,43 +121,22 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
122121
return AliasResult::MayAlias;
123122
}
124123

125-
// If we have reached the same source but comparing box reference against
126-
// data we are not comparing apples-to-apples. The 2 cannot alias.
127-
if ((lhsSrc.origin.u == rhsSrc.origin.u) &&
128-
lhsSrc.isData() != rhsSrc.isData()) {
129-
return AliasResult::NoAlias;
130-
}
131-
132124
if (lhsSrc.kind == rhsSrc.kind) {
133125
if (lhsSrc.origin == rhsSrc.origin) {
126+
LLVM_DEBUG(llvm::dbgs()
127+
<< " aliasing because same source kind and origin\n");
134128
if (approximateSource)
135129
return AliasResult::MayAlias;
136130
return AliasResult::MustAlias;
137131
}
138132

139133
// Two host associated accesses may overlap due to an equivalence.
140-
if (lhsSrc.kind == SourceKind::HostAssoc)
141-
return AliasResult::MayAlias;
142-
143-
// TARGET/POINTER arguments may alias.
144-
if (lhsSrc.isTargetOrPointer() && rhsSrc.isTargetOrPointer() &&
145-
lhsSrc.isData() == rhsSrc.isData())
134+
if (lhsSrc.kind == SourceKind::HostAssoc) {
135+
LLVM_DEBUG(llvm::dbgs() << " aliasing because of host association\n");
146136
return AliasResult::MayAlias;
147-
148-
// Box for POINTER component inside an object of a derived type
149-
// may alias box of a POINTER object, as well as boxes for POINTER
150-
// components inside two objects of derived types may alias.
151-
if ((lhsSrc.isRecordWithPointerComponent() && rhsSrc.isTargetOrPointer()) ||
152-
(rhsSrc.isRecordWithPointerComponent() && lhsSrc.isTargetOrPointer()) ||
153-
(lhsSrc.isRecordWithPointerComponent() &&
154-
rhsSrc.isRecordWithPointerComponent()))
155-
return AliasResult::MayAlias;
156-
157-
return AliasResult::NoAlias;
137+
}
158138
}
159139

160-
assert(lhsSrc.kind != rhsSrc.kind && "memory source kinds must be different");
161-
162140
Source *src1, *src2;
163141
if (lhsSrc.kind < rhsSrc.kind) {
164142
src1 = &lhsSrc;
@@ -190,17 +168,21 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
190168

191169
// Dummy TARGET/POINTER argument may alias with a global TARGET/POINTER.
192170
if (src1->isTargetOrPointer() && src2->isTargetOrPointer() &&
193-
src1->isData() == src2->isData())
171+
src1->isData() == src2->isData()) {
172+
LLVM_DEBUG(llvm::dbgs() << " aliasing because of target or pointer\n");
194173
return AliasResult::MayAlias;
174+
}
195175

196176
// Box for POINTER component inside an object of a derived type
197177
// may alias box of a POINTER object, as well as boxes for POINTER
198178
// components inside two objects of derived types may alias.
199179
if ((src1->isRecordWithPointerComponent() && src2->isTargetOrPointer()) ||
200180
(src2->isRecordWithPointerComponent() && src1->isTargetOrPointer()) ||
201181
(src1->isRecordWithPointerComponent() &&
202-
src2->isRecordWithPointerComponent()))
182+
src2->isRecordWithPointerComponent())) {
183+
LLVM_DEBUG(llvm::dbgs() << " aliasing because of pointer components\n");
203184
return AliasResult::MayAlias;
185+
}
204186

205187
return AliasResult::NoAlias;
206188
}
@@ -306,17 +288,15 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
306288
breakFromLoop = true;
307289
})
308290
.Case<fir::LoadOp>([&](auto op) {
309-
if (followBoxData && mlir::isa<fir::BaseBoxType>(op.getType())) {
310-
// For now, support the load of an argument or fir.address_of
311-
// TODO: generalize to all operations (in particular fir.alloca and
312-
// fir.allocmem)
313-
auto def = getOriginalDef(op.getMemref());
314-
if (isDummyArgument(def) ||
315-
def.template getDefiningOp<fir::AddrOfOp>()) {
316-
v = def;
317-
defOp = v.getDefiningOp();
318-
return;
319-
}
291+
// If the load is from a leaf source, return the leaf. Do not track
292+
// through indirections otherwise.
293+
// TODO: At support to fir.alloca and fir.allocmem
294+
auto def = getOriginalDef(op.getMemref());
295+
if (isDummyArgument(def) ||
296+
def.template getDefiningOp<fir::AddrOfOp>()) {
297+
v = def;
298+
defOp = v.getDefiningOp();
299+
return;
320300
}
321301
// No further tracking for addresses loaded from memory for now.
322302
type = SourceKind::Indirect;

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// RUN: fir-opt %s -pass-pipeline='builtin.module(func.func(test-fir-alias-analysis))' 2>&1 | FileCheck %s
2+
3+
4+
// module m
5+
// type t
6+
// type(t), pointer :: next
7+
// integer :: i
8+
// end type
9+
// contains
10+
// subroutine foo(x, y)
11+
// type(t) :: x, y
12+
// integer :: i1, i2
13+
// i1 = x%next%i
14+
// x = y
15+
// i2 = x%next%i
16+
// end subroutine
17+
// end module
18+
19+
// CHECK-LABEL: Testing : "_QMmPfoo"
20+
// TODO: x and y do not alias
21+
// CHECK-DAG: x#0 <-> y#0: MayAlias
22+
// CHECK-DAG: y#0 <-> xnext1#0: MayAlias
23+
// CHECK-DAG: y#0 <-> xnext2#0: MayAlias
24+
25+
// These however do alias
26+
// CHECK-DAG: x#0 <-> xnext1#0: MayAlias
27+
// CHECK-DAG: x#0 <-> xnext2#0: MayAlias
28+
// CHECK-DAG: xnext1#0 <-> xnext2#0: MayAlias
29+
30+
func.func @_QMmPfoo(%arg0: !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>> {fir.bindc_name = "x"}, %arg1: !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>> {fir.bindc_name = "y"}) {
31+
%0 = fir.alloca i32 {bindc_name = "i1", uniq_name = "_QMmFfooEi1"}
32+
%1:2 = hlfir.declare %0 {uniq_name = "_QMmFfooEi1"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
33+
%2 = fir.alloca i32 {bindc_name = "i2", uniq_name = "_QMmFfooEi2"}
34+
%3:2 = hlfir.declare %2 {uniq_name = "_QMmFfooEi2"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
35+
%4:2 = hlfir.declare %arg0 {uniq_name = "_QMmFfooEx", test.ptr = "x"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>, !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>)
36+
%5:2 = hlfir.declare %arg1 {uniq_name = "_QMmFfooEy", test.ptr = "y"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>, !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>)
37+
%6 = hlfir.designate %4#0{"next"} {fortran_attrs = #fir.var_attrs<pointer>, test.ptr = "xnext1"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> !fir.ref<!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>>
38+
%7 = fir.load %6 : !fir.ref<!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>>
39+
%8 = fir.box_addr %7 : (!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>) -> !fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>
40+
%9 = hlfir.designate %8{"i"} : (!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> !fir.ref<i32>
41+
%10 = fir.load %9 : !fir.ref<i32>
42+
hlfir.assign %10 to %1#0 : i32, !fir.ref<i32>
43+
hlfir.assign %5#0 to %4#0 : !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>, !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>
44+
%11 = hlfir.designate %4#0{"next"} {fortran_attrs = #fir.var_attrs<pointer>, test.ptr = "xnext2"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> !fir.ref<!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>>
45+
%12 = fir.load %11 : !fir.ref<!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>>
46+
%13 = fir.box_addr %12 : (!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>) -> !fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>
47+
%14 = hlfir.designate %13{"i"} : (!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> !fir.ref<i32>
48+
%15 = fir.load %14 : !fir.ref<i32>
49+
hlfir.assign %15 to %3#0 : i32, !fir.ref<i32>
50+
return
51+
}

0 commit comments

Comments
 (0)