Skip to content

Commit 65f9d6c

Browse files
vzakharivzakhari
committed
Turned PartialAlias into MayAlias as conservatively correct.
Fixed the visitation to guarantee that the order does not matter.
1 parent 36577e3 commit 65f9d6c

File tree

5 files changed

+97
-81
lines changed

5 files changed

+97
-81
lines changed

‎flang/test/Analysis/AliasAnalysis/load-ptr-designate.fir‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@
8888
// CHECK-DAG: obj#0 <-> obj%alloc#0: MayAlias
8989
// CHECK-DAG: obj#0 <-> obj%p0.tgt#0: MayAlias
9090
// CHECK-DAG: obj#0 <-> obj%alloc.tgt#0: MayAlias
91-
// CHECK-DAG: obj.fir#0 <-> obj%p0.fir#0: PartialAlias
92-
// CHECK-DAG: obj.fir#0 <-> obj%alloc.fir#0: PartialAlias
91+
// CHECK-DAG: obj.fir#0 <-> obj%p0.fir#0: MayAlias
92+
// CHECK-DAG: obj.fir#0 <-> obj%alloc.fir#0: MayAlias
9393
// CHECK-DAG: obj.fir#0 <-> obj%p0.tgt.fir#0: MayAlias
9494
// CHECK-DAG: obj.fir#0 <-> obj%alloc.tgt.fir#0: MayAlias
9595

@@ -252,8 +252,8 @@ func.func @_QPtest.fir() {
252252
// CHECK-DAG: obj#0 <-> obj%alloc#0: MayAlias
253253
// CHECK-DAG: obj#0 <-> obj%p0.tgt#0: MayAlias
254254
// CHECK-DAG: obj#0 <-> obj%alloc.tgt#0: MayAlias
255-
// CHECK-DAG: obj.fir#0 <-> obj%p0.fir#0: PartialAlias
256-
// CHECK-DAG: obj.fir#0 <-> obj%alloc.fir#0: PartialAlias
255+
// CHECK-DAG: obj.fir#0 <-> obj%p0.fir#0: MayAlias
256+
// CHECK-DAG: obj.fir#0 <-> obj%alloc.fir#0: MayAlias
257257
// CHECK-DAG: obj.fir#0 <-> obj%p0.tgt.fir#0: MayAlias
258258
// CHECK-DAG: obj.fir#0 <-> obj%alloc.tgt.fir#0: MayAlias
259259

@@ -412,8 +412,8 @@ func.func @_QPtest.fir(%arg0: !fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f3
412412
// CHECK-DAG: obj#0 <-> obj%alloc#0: MayAlias
413413
// CHECK-DAG: obj#0 <-> obj%p0.tgt#0: MayAlias
414414
// CHECK-DAG: obj#0 <-> obj%alloc.tgt#0: MayAlias
415-
// CHECK-DAG: obj.fir#0 <-> obj%p0.fir#0: PartialAlias
416-
// CHECK-DAG: obj.fir#0 <-> obj%alloc.fir#0: PartialAlias
415+
// CHECK-DAG: obj.fir#0 <-> obj%p0.fir#0: MayAlias
416+
// CHECK-DAG: obj.fir#0 <-> obj%alloc.fir#0: MayAlias
417417
// CHECK-DAG: obj.fir#0 <-> obj%p0.tgt.fir#0: MayAlias
418418
// CHECK-DAG: obj.fir#0 <-> obj%alloc.tgt.fir#0: MayAlias
419419

‎flang/test/Analysis/AliasAnalysis/ptr-component.fir‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,13 @@
4141
// therefore x needs to alias with x.next to prevent the loads from being merged.
4242
// CHECK-DAG: x#0 <-> xnext1#0: MayAlias
4343
// CHECK-DAG: x#0 <-> xnext2#0: MayAlias
44-
// CHECK-DAG: x.fir#0 <-> xnext1.fir#0: PartialAlias
45-
// CHECK-DAG: x.fir#0 <-> xnext2.fir#0: PartialAlias
44+
// CHECK-DAG: x.fir#0 <-> xnext1.fir#0: MayAlias
45+
// CHECK-DAG: x.fir#0 <-> xnext2.fir#0: MayAlias
4646

4747
// TODO: xnext1#0 <-> xnext2#0 are the same and therefore MustAlias but
4848
// we are currently not comparing operands involved in offset computations
4949
// CHECK-DAG: xnext1#0 <-> xnext2#0: MayAlias
50-
// CHECK-DAG: xnext1.fir#0 <-> xnext2.fir#0: PartialAlias
50+
// CHECK-DAG: xnext1.fir#0 <-> xnext2.fir#0: MayAlias
5151

5252
// CHECK-DAG: xnext1#0 <-> ynext#0: NoAlias
5353
// CHECK-DAG: xnext1.fir#0 <-> ynext.fir#0: NoAlias
@@ -217,7 +217,7 @@ func.func @_QMmPfoo3.fir(%arg0: !fir.ref<!fir.type<_QMmTta{array:!fir.box<!fir.p
217217

218218
// CHECK-LABEL: Testing : "_QMmPtest"
219219
// CHECK-DAG: x#0 <-> x%p#0: MayAlias
220-
// CHECK-DAG: x.fir#0 <-> x%p.fir#0: PartialAlias
220+
// CHECK-DAG: x.fir#0 <-> x%p.fir#0: MayAlias
221221

222222
func.func @_QMmPtest() {
223223
%0 = fir.address_of(@_QMmEx) : !fir.ref<!fir.type<_QMmTt{p:!fir.box<!fir.heap<i32>>}>>
@@ -258,8 +258,8 @@ func.func @_QMmPtest.fir() {
258258
// composite is nested within another composite.
259259
// CHECK-DAG: x#0 <-> x%p#0: MayAlias
260260
// CHECK-DAG: x%x#0 <-> x%x%p#0: MayAlias
261-
// CHECK-DAG: x.fir#0 <-> x%p.fir#0: PartialAlias
262-
// CHECK-DAG: x%x.fir#0 <-> x%x%p.fir#0: PartialAlias
261+
// CHECK-DAG: x.fir#0 <-> x%p.fir#0: MayAlias
262+
// CHECK-DAG: x%x.fir#0 <-> x%x%p.fir#0: MayAlias
263263

264264
// The addresses of different components of the same composite do not alias.
265265
//
@@ -274,12 +274,9 @@ func.func @_QMmPtest.fir() {
274274
// CHECK-DAG: x%x#0 <-> x%i#0: MayAlias
275275
// CHECK-DAG: x%p#0 <-> x%i#0: NoAlias
276276
// CHECK-DAG: x%x#0 <-> x%p#0: MayAlias
277-
// CHECK-DAG: x%x.fir#0 <-> x%i.fir#0: PartialAlias
278-
279-
// NOTE: FIR AliasAnalysis can prove NoAlias, but LocalAliasAnalysis
280-
// conservatively assumes PartialAlias.
281-
// CHECK-DAG: x%p.fir#0 <-> x%i.fir#0: PartialAlias
282-
// CHECK-DAG: x%x.fir#0 <-> x%p.fir#0: PartialAlias
277+
// CHECK-DAG: x%x.fir#0 <-> x%i.fir#0: MayAlias
278+
// CHECK-DAG: x%p.fir#0 <-> x%i.fir#0: NoAlias
279+
// CHECK-DAG: x%x.fir#0 <-> x%p.fir#0: MayAlias
283280

284281
func.func @_QMmPtest() {
285282
%0 = fir.alloca !fir.type<_QMmTt2{x:!fir.type<_QMmTt1{p:!fir.box<!fir.ptr<i32>>}>,p:!fir.box<!fir.ptr<i32>>,i:i32}> {bindc_name = "x", uniq_name = "_QMmFtestEx"}

‎mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ static constexpr unsigned maxUnderlyingValueSearchDepth = 10;
4444
/// with regards to the memory object pointed to by the `value`.
4545
static void
4646
collectUnderlyingAddressValues(Value value, unsigned maxDepth,
47-
DenseSet<Value> &visited, bool maybeOffset,
47+
DenseMap<Value, bool> &visited, bool maybeOffset,
4848
SmallVectorImpl<std::pair<Value, bool>> &output);
4949

5050
/// Given a successor (`region`) of a RegionBranchOpInterface, collect all of
@@ -53,7 +53,7 @@ collectUnderlyingAddressValues(Value value, unsigned maxDepth,
5353
/// the parent operation.
5454
static void collectUnderlyingAddressValues(
5555
RegionBranchOpInterface branch, Region *region, Value inputValue,
56-
unsigned inputIndex, unsigned maxDepth, DenseSet<Value> &visited,
56+
unsigned inputIndex, unsigned maxDepth, DenseMap<Value, bool> &visited,
5757
bool maybeOffset, SmallVectorImpl<std::pair<Value, bool>> &output) {
5858
// Given the index of a region of the branch (`predIndex`), or std::nullopt to
5959
// represent the parent operation, try to return the index into the outputs of
@@ -125,7 +125,7 @@ static void collectUnderlyingAddressValues(
125125

126126
/// Given a result, collect all of the underlying values being addressed.
127127
static void collectUnderlyingAddressValues(
128-
OpResult result, unsigned maxDepth, DenseSet<Value> &visited,
128+
OpResult result, unsigned maxDepth, DenseMap<Value, bool> &visited,
129129
bool maybeOffset, SmallVectorImpl<std::pair<Value, bool>> &output) {
130130
Operation *op = result.getOwner();
131131

@@ -149,7 +149,7 @@ static void collectUnderlyingAddressValues(
149149
/// Given a block argument, collect all of the underlying values being
150150
/// addressed.
151151
static void collectUnderlyingAddressValues(
152-
BlockArgument arg, unsigned maxDepth, DenseSet<Value> &visited,
152+
BlockArgument arg, unsigned maxDepth, DenseMap<Value, bool> &visited,
153153
bool maybeOffset, SmallVectorImpl<std::pair<Value, bool>> &output) {
154154
Block *block = arg.getOwner();
155155
unsigned argNumber = arg.getArgNumber();
@@ -192,11 +192,21 @@ static void collectUnderlyingAddressValues(
192192

193193
/// Given a value, collect all of the underlying values being addressed.
194194
static void collectUnderlyingAddressValues(
195-
Value value, unsigned maxDepth, DenseSet<Value> &visited, bool maybeOffset,
196-
SmallVectorImpl<std::pair<Value, bool>> &output) {
195+
Value value, unsigned maxDepth, DenseMap<Value, bool> &visited,
196+
bool maybeOffset, SmallVectorImpl<std::pair<Value, bool>> &output) {
197197
// Check that we don't infinitely recurse.
198-
if (!visited.insert(value).second)
199-
return;
198+
auto it = visited.find(value);
199+
if (it != visited.end()) {
200+
// If maybeOffset is true, we have to propagate it up
201+
// the operands chain. If the value has already been visited
202+
// with a false maybeOffset, we have to visit it again.
203+
if (!maybeOffset || it->second)
204+
return;
205+
206+
it->second = true;
207+
} else {
208+
visited.try_emplace(value, maybeOffset);
209+
}
200210
if (maxDepth == 0) {
201211
output.push_back({value, maybeOffset});
202212
return;
@@ -213,7 +223,7 @@ static void collectUnderlyingAddressValues(
213223
/// Given a value, collect all of the underlying values being addressed.
214224
static void collectUnderlyingAddressValues(
215225
Value value, SmallVectorImpl<std::pair<Value, bool>> &output) {
216-
DenseSet<Value> visited;
226+
DenseMap<Value, bool> visited;
217227
collectUnderlyingAddressValues(value, maxUnderlyingValueSearchDepth, visited,
218228
/*maybeOffset=*/false, output);
219229
}
@@ -392,9 +402,12 @@ AliasResult LocalAliasAnalysis::alias(Value lhs, Value rhs) {
392402
AliasResult nextResult = aliasImpl(lhsVal, rhsVal);
393403
// If the original lhs/rhs may be an offsetted access
394404
// of the memory objects pointed to by lhsVal/rhsVal,
395-
// we'd better turn MustAlias results into PartialAlias.
405+
// we'd better turn MustAlias results into MayAlias.
406+
// It is possible that MustAlias and PartialAlias
407+
// are both dynamically possible for these two references,
408+
// so MayAlias should be conservatively correct.
396409
if (nextResult == AliasResult::MustAlias && (lhsPartial || rhsPartial))
397-
nextResult = AliasResult::PartialAlias;
410+
nextResult = AliasResult::MayAlias;
398411
result = result ? result->merge(nextResult) : nextResult;
399412
}
400413
}

‎mlir/test/Analysis/test-alias-analysis-extending.mlir‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66
// CHECK-DAG: view1#0 <-> view2#0: NoAlias
77
// CHECK-DAG: view1#0 <-> func.region0#0: MustAlias
88
// CHECK-DAG: view1#0 <-> func.region0#1: NoAlias
9-
// CHECK-DAG: view1#0 <-> view3#0: PartialAlias
9+
// CHECK-DAG: view1#0 <-> view3#0: MayAlias
1010
// CHECK-DAG: view1#0 <-> view4#0: NoAlias
1111
// CHECK-DAG: view2#0 <-> func.region0#0: NoAlias
1212
// CHECK-DAG: view2#0 <-> func.region0#1: MustAlias
1313
// CHECK-DAG: view2#0 <-> view3#0: NoAlias
14-
// CHECK-DAG: view2#0 <-> view4#0: PartialAlias
15-
// CHECK-DAG: view3#0 <-> func.region0#0: PartialAlias
14+
// CHECK-DAG: view2#0 <-> view4#0: MayAlias
15+
// CHECK-DAG: view3#0 <-> func.region0#0: MayAlias
1616
// CHECK-DAG: view3#0 <-> func.region0#1: NoAlias
1717
// CHECK-DAG: view3#0 <-> view4#0: NoAlias
1818
func.func @restrict(%arg: memref<?xf32>, %arg1: memref<?xf32> {local_alias_analysis.restrict}) attributes {test.ptr = "func"} {

‎mlir/test/Analysis/test-alias-analysis.mlir‎

Lines changed: 55 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -279,18 +279,18 @@ func.func @distinct_objects(%arg: memref<?xf32>, %arg1: memref<?xf32>) attribute
279279

280280
// The current LocalAliasAnalysis algortithm is quite conservative
281281
// about differentiating PartialAlias and MustAlias, and it often
282-
// returns PartialAlias for MustAlias situations.
283-
284-
// These are the PartialAlias cases, where LocalAliasAnalysis
285-
// used to incorrectly return MustAlias:
286-
// CHECK-DAG: view_with_offset#0 <-> func.region0#0: PartialAlias
287-
// CHECK-DAG: complete_view1#0 <-> func.region0#0: PartialAlias
288-
// CHECK-DAG: complete_view2#0 <-> func.region0#0: PartialAlias
289-
290-
// TODO: these are the MustAlias cases, where PartialAlias is returned currently:
291-
// CHECK-DAG: view_with_offset#0 <-> complete_view1#0: PartialAlias
292-
// CHECK-DAG: view_with_offset#0 <-> complete_view2#0: PartialAlias
293-
// CHECK-DAG: complete_view1#0 <-> complete_view2#0: PartialAlias
282+
// returns MayAlias for PartialAlias/MustAlias situations.
283+
284+
// These are the PartialAlias cases (reported as MayAlias),
285+
// where LocalAliasAnalysis used to incorrectly return MustAlias:
286+
// CHECK-DAG: view_with_offset#0 <-> func.region0#0: MayAlias
287+
// CHECK-DAG: complete_view1#0 <-> func.region0#0: MayAlias
288+
// CHECK-DAG: complete_view2#0 <-> func.region0#0: MayAlias
289+
290+
// These are the MustAlias cases, where MayAlias is returned currently:
291+
// CHECK-DAG: view_with_offset#0 <-> complete_view1#0: MayAlias
292+
// CHECK-DAG: view_with_offset#0 <-> complete_view2#0: MayAlias
293+
// CHECK-DAG: complete_view1#0 <-> complete_view2#0: MayAlias
294294
func.func @view_like_offset(%arg: memref<?xi8>) attributes {test.ptr = "func"} {
295295
%c0 = arith.constant 0 : index
296296
%c1 = arith.constant 1 : index
@@ -304,27 +304,30 @@ func.func @view_like_offset(%arg: memref<?xi8>) attributes {test.ptr = "func"} {
304304

305305
// CHECK-LABEL: Testing : "view_like_offset_with_cfg1"
306306

307-
// These are the PartialAlias cases, where LocalAliasAnalysis
308-
// used to incorrectly return MustAlias:
309-
// CHECK-DAG: complete_view_if#0 <-> view_with_offset#0: PartialAlias
310-
// CHECK-DAG: view_with_offset#0 <-> if_view#0: PartialAlias
311-
// CHECK-DAG: view_with_offset#0 <-> complete_view1#0: PartialAlias
312-
// CHECK-DAG: view_with_offset#0 <-> complete_view2#0: PartialAlias
313-
// CHECK-DAG: view_with_offset#0 <-> func.region0#0: PartialAlias
307+
// These are the PartialAlias cases (reported as MayAlias),
308+
// where LocalAliasAnalysis used to incorrectly return MustAlias:
309+
// CHECK-DAG: complete_view_if#0 <-> view_with_offset#0: MayAlias
310+
// CHECK-DAG: view_with_offset#0 <-> if_view#0: MayAlias
311+
// CHECK-DAG: view_with_offset#0 <-> complete_view1#0: MayAlias
312+
// CHECK-DAG: view_with_offset#0 <-> complete_view2#0: MayAlias
313+
// CHECK-DAG: view_with_offset#0 <-> func.region0#0: MayAlias
314314

315315
// These are correctly classified as MustAlias:
316-
// CHECK-DAG: if_view#0 <-> complete_view1#0: MustAlias
317-
// CHECK-DAG: if_view#0 <-> complete_view2#0: MustAlias
318-
// CHECK-DAG: complete_view1#0 <-> complete_view2#0: MustAlias
319316
// CHECK-DAG: complete_view_if#0 <-> func.region0#0: MustAlias
320317

321-
// TODO: these should be either PartialAlias or MayAlias:
322-
// CHECK-DAG: complete_view_if#0 <-> if_view#0: MustAlias
323-
// CHECK-DAG: complete_view_if#0 <-> complete_view1#0: MustAlias
324-
// CHECK-DAG: complete_view_if#0 <-> complete_view2#0: MustAlias
325-
// CHECK-DAG: if_view#0 <-> func.region0#0: MustAlias
326-
// CHECK-DAG: complete_view1#0 <-> func.region0#0: MustAlias
327-
// CHECK-DAG: complete_view2#0 <-> func.region0#0: MustAlias
318+
// TODO: these must be MustAlias:
319+
// CHECK-DAG: if_view#0 <-> complete_view1#0: MayAlias
320+
// CHECK-DAG: if_view#0 <-> complete_view2#0: MayAlias
321+
// CHECK-DAG: complete_view1#0 <-> complete_view2#0: MayAlias
322+
323+
// These should be MayAlias, because the references are either
324+
// MustAlias or PartialAlias depending on the runtime condition:
325+
// CHECK-DAG: complete_view_if#0 <-> if_view#0: MayAlias
326+
// CHECK-DAG: complete_view_if#0 <-> complete_view1#0: MayAlias
327+
// CHECK-DAG: complete_view_if#0 <-> complete_view2#0: MayAlias
328+
// CHECK-DAG: if_view#0 <-> func.region0#0: MayAlias
329+
// CHECK-DAG: complete_view1#0 <-> func.region0#0: MayAlias
330+
// CHECK-DAG: complete_view2#0 <-> func.region0#0: MayAlias
328331
func.func @view_like_offset_with_cfg1(%arg: memref<?xi8>, %cond: i1) attributes {test.ptr = "func"} {
329332
%c0 = arith.constant 0 : index
330333
%c1 = arith.constant 1 : index
@@ -346,30 +349,33 @@ func.func @view_like_offset_with_cfg1(%arg: memref<?xi8>, %cond: i1) attributes
346349

347350
// This test is a different version of view_like_offset_with_cfg1:
348351
// the then and else clauses are swapped, and this affects
349-
// the visiting order causing different PartialAlias vs MustAlias
350-
// reports.
351-
352-
// These are the PartialAlias cases, where LocalAliasAnalysis
353-
// used to incorrectly return MustAlias:
354-
// CHECK-DAG: view_with_offset#0 <-> complete_view_if#0: PartialAlias
355-
// CHECK-DAG: view_with_offset#0 <-> if_view#0: PartialAlias
356-
// CHECK-DAG: complete_view_if#0 <-> if_view#0: PartialAlias
357-
// CHECK-DAG: view_with_offset#0 <-> complete_view1#0: PartialAlias
358-
// CHECK-DAG: complete_view_if#0 <-> complete_view1#0: PartialAlias
359-
// CHECK-DAG: view_with_offset#0 <-> complete_view2#0: PartialAlias
360-
// CHECK-DAG: complete_view_if#0 <-> complete_view2#0: PartialAlias
361-
// CHECK-DAG: view_with_offset#0 <-> func.region0#0: PartialAlias
362-
// CHECK-DAG: if_view#0 <-> func.region0#0: PartialAlias
363-
// CHECK-DAG: complete_view1#0 <-> func.region0#0: PartialAlias
364-
// CHECK-DAG: complete_view2#0 <-> func.region0#0: PartialAlias
352+
// the visiting order. The two tests check that the visiting
353+
// order does not matter.
354+
355+
// These are the PartialAlias cases (reported as MayAlias),
356+
// where LocalAliasAnalysis used to incorrectly return MustAlias:
357+
// CHECK-DAG: view_with_offset#0 <-> complete_view_if#0: MayAlias
358+
// CHECK-DAG: view_with_offset#0 <-> if_view#0: MayAlias
359+
// CHECK-DAG: view_with_offset#0 <-> complete_view1#0: MayAlias
360+
// CHECK-DAG: if_view#0 <-> func.region0#0: MayAlias
365361

366362
// These are correctly classified as MustAlias:
367363
// CHECK-DAG: complete_view_if#0 <-> func.region0#0: MustAlias
368364

369-
// TODO: these are the MustAlias cases, where PartialAlias is returned currently:
370-
// CHECK-DAG: if_view#0 <-> complete_view1#0: PartialAlias
371-
// CHECK-DAG: if_view#0 <-> complete_view2#0: PartialAlias
372-
// CHECK-DAG: complete_view1#0 <-> complete_view2#0: PartialAlias
365+
// TODO: these must be MustAlias:
366+
// CHECK-DAG: if_view#0 <-> complete_view1#0: MayAlias
367+
// CHECK-DAG: if_view#0 <-> complete_view2#0: MayAlias
368+
// CHECK-DAG: complete_view1#0 <-> complete_view2#0: MayAlias
369+
370+
// These should be MayAlias, because the references are either
371+
// MustAlias or PartialAlias depending on the runtime condition:
372+
// CHECK-DAG: complete_view_if#0 <-> if_view#0: MayAlias
373+
// CHECK-DAG: complete_view_if#0 <-> complete_view1#0: MayAlias
374+
// CHECK-DAG: view_with_offset#0 <-> complete_view2#0: MayAlias
375+
// CHECK-DAG: complete_view_if#0 <-> complete_view2#0: MayAlias
376+
// CHECK-DAG: view_with_offset#0 <-> func.region0#0: MayAlias
377+
// CHECK-DAG: complete_view1#0 <-> func.region0#0: MayAlias
378+
// CHECK-DAG: complete_view2#0 <-> func.region0#0: MayAlias
373379
func.func @view_like_offset_with_cfg2(%arg: memref<?xi8>, %cond: i1) attributes {test.ptr = "func"} {
374380
%c0 = arith.constant 0 : index
375381
%c1 = arith.constant 1 : index

0 commit comments

Comments
 (0)