Skip to content

Commit ee248f5

Browse files
committed
Fix todos for handling addresses of pointers
The general `if` calling `isTargetOrPointer` was handling addresses *of* pointers similarly to addresses *in* pointers: it assumed they may alias each other. While that's true in the case of dummy args, it's not true generally, as revealed by some of the test suite todos fixed by this commit.
1 parent 4a73682 commit ee248f5

File tree

2 files changed

+24
-34
lines changed

2 files changed

+24
-34
lines changed

flang/lib/Optimizer/Analysis/AliasAnalysis.cpp

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,13 @@ bool AliasAnalysis::Source::canBeActualArgWithPtr(
118118
// Must not be local.
119119
if (!canBeActualArg())
120120
return false;
121-
// Must be address of a composite with a pointer component.
122-
return isRecordWithPointerComponent(val->getType());
121+
// Can be address *of* (not *in*) a pointer.
122+
if (attributes.test(Attribute::Pointer) && !isData())
123+
return true;
124+
// Can be address of a composite with a pointer component.
125+
if (isRecordWithPointerComponent(val->getType()))
126+
return true;
127+
return false;
123128
}
124129

125130
AliasResult AliasAnalysis::alias(mlir::Value lhs, mlir::Value rhs) {
@@ -222,16 +227,18 @@ AliasResult AliasAnalysis::alias(mlir::Value lhs, mlir::Value rhs) {
222227
src2->attributes.set(Attribute::Target);
223228
}
224229

225-
// Two TARGET/POINTERs may alias.
230+
// Two TARGET/POINTERs may alias. The logic here focuses on data. Handling
231+
// of non-data is included below.
226232
if (src1->isTargetOrPointer() && src2->isTargetOrPointer() &&
227-
src1->isData() == src2->isData()) {
233+
src1->isData() && src2->isData()) {
228234
LLVM_DEBUG(llvm::dbgs() << " aliasing because of target or pointer\n");
229235
return AliasResult::MayAlias;
230236
}
231237

232-
// A pointer dummy arg (but not a pointer component of a dummy arg) may alias
233-
// a pointer component and thus the associated composite. That composite
234-
// might be a global or another dummy arg. This is an example of the global
238+
// The address of a pointer dummy arg (but not a pointer component of a dummy
239+
// arg) may alias the address of either (1) a non-local pointer or (2) thus a
240+
// non-local composite with a pointer component. A non-local might be a
241+
// global or another dummy arg. The following is an example of the global
235242
// composite case:
236243
//
237244
// module m
@@ -276,8 +283,8 @@ AliasResult AliasAnalysis::alias(mlir::Value lhs, mlir::Value rhs) {
276283
if ((src1->aliasesLikePtrDummyArg() && src2->canBeActualArgWithPtr(val2)) ||
277284
(src2->aliasesLikePtrDummyArg() && src1->canBeActualArgWithPtr(val1))) {
278285
LLVM_DEBUG(llvm::dbgs()
279-
<< " aliasing between pointer dummy arg and composite with "
280-
<< "pointer component\n");
286+
<< " aliasing between pointer dummy arg and either pointer or "
287+
<< "composite with pointer component\n");
281288
return AliasResult::MayAlias;
282289
}
283290

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

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,8 @@
5252
// CHECK-DAG: xnext1#0 <-> xnext2#0: MayAlias
5353
// CHECK-DAG: xnext1.fir#0 <-> xnext2.fir#0: MayAlias
5454

55-
// TODO: These should be NoAlias. However, AliasAnalysis currently
56-
// indiscriminately treats all pointers as aliasing. That makes sense for the
57-
// addresses within the pointers but not necessarily for the addresses of the
58-
// pointers.
59-
// CHECK-DAG: xnext1#0 <-> ynext#0: MayAlias
60-
// CHECK-DAG: xnext1.fir#0 <-> ynext.fir#0: MayAlias
55+
// CHECK-DAG: xnext1#0 <-> ynext#0: NoAlias
56+
// CHECK-DAG: xnext1.fir#0 <-> ynext.fir#0: NoAlias
6157

6258
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"}) {
6359
%0 = fir.alloca i32 {bindc_name = "i1", uniq_name = "_QMmFfooEi1"}
@@ -391,16 +387,11 @@ func.func @_QMmPtest.fir() {
391387

392388
// Check when composite is a local and thus cannot alias a dummy arg.
393389
//
394-
// TODO: The argp vs. loc%p cases should be NoAlias. However, AliasAnalysis
395-
// currently indiscriminately treats all pointers as aliasing. That makes sense
396-
// for the addresses within the pointers but not necessarily for the addresses
397-
// of the pointers here.
398-
//
399390
// CHECK-DAG: argp#0 <-> loc#0: NoAlias
400-
// CHECK-DAG: argp#0 <-> loc%p#0: MayAlias
391+
// CHECK-DAG: argp#0 <-> loc%p#0: NoAlias
401392
// CHECK-DAG: argp#0 <-> loc%i#0: NoAlias
402393
// CHECK-DAG: argp.fir#0 <-> loc.fir#0: NoAlias
403-
// CHECK-DAG: argp.fir#0 <-> loc%p.fir#0: MayAlias
394+
// CHECK-DAG: argp.fir#0 <-> loc%p.fir#0: NoAlias
404395
// CHECK-DAG: argp.fir#0 <-> loc%i.fir#0: NoAlias
405396
//
406397
// CHECK-DAG: argp.tgt#0 <-> loc#0: NoAlias
@@ -519,13 +510,10 @@ func.func @_QMmPtest.fir(%arg0: !fir.ref<!fir.box<!fir.ptr<i32>>> {fir.bindc_nam
519510
// CHECK-DAG: argp.tgt.fir#0 <-> arg%p.fir#0: MayAlias
520511
// CHECK-DAG: argp.tgt.fir#0 <-> arg%i.fir#0: MayAlias
521512
//
522-
// TODO: Shouldn't the arga vs. arg%p cases be NoAlias? However, arga is
523-
// treated like a target because it's HostAssoc and arg is Argument, and arg%p
524-
// is treated like a pointer.
525513
// CHECK-DAG: arga#0 <-> arg#0: NoAlias
526-
// CHECK-DAG: arga#0 <-> arg%p#0: MayAlias
514+
// CHECK-DAG: arga#0 <-> arg%p#0: NoAlias
527515
// CHECK-DAG: arga.fir#0 <-> arg.fir#0: NoAlias
528-
// CHECK-DAG: arga.fir#0 <-> arg%p.fir#0: MayAlias
516+
// CHECK-DAG: arga.fir#0 <-> arg%p.fir#0: NoAlias
529517

530518
// Check when composite is a global.
531519
//
@@ -552,16 +540,11 @@ func.func @_QMmPtest.fir(%arg0: !fir.ref<!fir.box<!fir.ptr<i32>>> {fir.bindc_nam
552540

553541
// Check when composite is a local and thus cannot alias a dummy arg.
554542
//
555-
// TODO: The argp vs. loc%p case should be NoAlias. However, AliasAnalysis
556-
// currently indiscriminately treats all pointers as aliasing. That makes sense
557-
// for the addresses within the pointers but not necessarily for the addresses
558-
// of the pointers here.
559-
//
560543
// CHECK-DAG: argp#0 <-> loc#0: NoAlias
561-
// CHECK-DAG: argp#0 <-> loc%p#0: MayAlias
544+
// CHECK-DAG: argp#0 <-> loc%p#0: NoAlias
562545
// CHECK-DAG: argp#0 <-> loc%i#0: NoAlias
563546
// CHECK-DAG: argp.fir#0 <-> loc.fir#0: NoAlias
564-
// CHECK-DAG: argp.fir#0 <-> loc%p.fir#0: MayAlias
547+
// CHECK-DAG: argp.fir#0 <-> loc%p.fir#0: NoAlias
565548
// CHECK-DAG: argp.fir#0 <-> loc%i.fir#0: NoAlias
566549
//
567550
// TODO: Shouldn't these be NoAlias? However, argp.tgt is currently handled as

0 commit comments

Comments
 (0)