[flang-commits] [flang] [flang] AliasAnalysis: Handle fir.load on fir.alloca (PR #117785)

[Joel E. Denny via flang-commits]

================
@@ -522,6 +546,12 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
         .Case<fir::AllocaOp, fir::AllocMemOp>([&](auto op) {
           // Unique memory allocation.
           type = SourceKind::Allocate;
+          // If there's no DeclareOp, then we need to get the pointer attribute
+          // from the type.  TODO: That case occurs in our test suite
+          // (alias-analysis-2.fir), but does flang currently generate such
+          // code?
+          if (isPointerReference(ty))
+            attributes.set(Attribute::Pointer);
----------------
jdenny-ornl wrote:

Thanks for your reply.  In this thread, we've discussed three changes to AliasAnalysis.cpp:

- C1: [unset followingData on AllocaOp](https://github.com/jdenny-ornl/llvm-project/blob/00aeefe3a5fb1944b7b6be0098edb1916d77be9a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp#L546-L556) (included in the PR)
- C2: [add Attribute::Pointer to AllocaOp](https://github.com/jdenny-ornl/llvm-project/blob/00aeefe3a5fb1944b7b6be0098edb1916d77be9a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp#L570-L575) (included in the PR)
- C3: [add Attribute::Pointer to BoxAddrOp](https://github.com/llvm/llvm-project/pull/117785#discussion_r1859666490) (your suggestion)

There are two checks in alias-analysis-2.fir that sometimes break depending on which of the above changes we have while keeping all remaining changes from this PR:

- T1: [p1.addr#0 <-> p2.addr#0: NoAlias](https://github.com/jdenny-ornl/llvm-project/blob/00aeefe3a5fb1944b7b6be0098edb1916d77be9a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir#L9)
- T2: [boxp1.addr#0 <-> func.region0#0: MayAlias](https://github.com/jdenny-ornl/llvm-project/blob/00aeefe3a5fb1944b7b6be0098edb1916d77be9a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir#L34C1-L34C56)

T2 breaks when none of the three changes are applied because boxp1.addr is not treated as a pointer.  Each of C2 and C3 fixes that.

T1 breaks when C2 is applied without C1 (regardless of whether C3 is applied) because C2 causes the address of a pointer without a box to be mislabeled as pointer data instead of pointer non-data.

So, for check-flang to pass, we seem to need either C1+C2 (PR as it is now), C3 alone, or C1+C2+C3.  I am concerned about C3 alone due to points 1 and 3 in [my previous comment](https://github.com/llvm/llvm-project/pull/117785#discussion_r1860967421).  I don't know what cases C1+C2+C3 handles that C1+C2 doesn't, but we should extend the test suite to cover them if we go with that.

> 1. We introduced data vs non-data, to lift ambiguity when following a box and to distinguish between the box's address or the address it wraps.

Understood.

> There is no such ambiguity for scalars, so the classical way of distinguishing between 2 sources should work.

I'm not sure which case you are referring to when you mention scalars. As far as I know, the relevant cases (T1 and T2) involve addresses of/in pointers, sometimes with boxes.

> 2. After discussing with Jean, all the pointers are boxed since HLFIR, but to optimize the code some may be reverted back to just allocas. It is best to keep options open.

OK, so it seems we have to keep at least one of the combinations listed above.

> 3. You may keep the change at the lines 553,554 (where this discussion is anchored), it is not harmful and consistent with the global case, as you point out.

That's C2.  I think you're suggesting we go with C2+C3, but that breaks T1, as described above.

https://github.com/llvm/llvm-project/pull/117785