[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:

> After my changes, C1 to C3 will probably look very different. It will not make sense to discuss them.  Could be best to start a new PR.
>
> As far as discussing T3. You should be driving the discussion maybe even file an issue. But because of the time spent, my management is now asking for benchmark data so be ready for that.

I have data to show how this PR plus one more I haven't proposed yet significantly improved performance of an app we are investigating. The data is stale by so many months now, I should probably run it again before posting it.  I haven't proposed the other PR yet because I suspect I'll have to rewrite it by the time this PR lands, as your comment above also suggests.

However, the vast majority of the discussion for this PR (including C1, C3, T1, T2, T3, and T4) has been for `fir.alloca !fir.ptr<T>` and `!fir.ref<!fir.ptr<T>>`, a case that, so far in this discussion, no one has made Flang produce for any Fortran program.  But you and @jeanPerier say it matters.  If we need to justify all the time we're spending on this PR, perhaps one of you can show when that case matters.  I certainly don't have benchmark data for it.

> The 80+ other original tests look good. We can move forward with them. But this should just be a matter of generalizing what we are doing in the box load since, from what I could tell, it is what is limiting these tests. We do need to change our definition of data vs non-data for them.

Sounds good.  Thanks for your help.

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