[flang] AliasAnalysis: Fix pointer component logic (llvm#94242)

This PR applies the changes discussed in [[RFC] Rationale for Flang
AliasAnalysis pointer component
logic](https://discourse.llvm.org/t/rfc-rationale-for-flang-aliasanalysis-pointer-component-logic/79252).

In summary, this PR replaces the existing pointer component logic in
Flang's AliasAnalysis implementation. That logic focuses on aliasing
between pointers and non-pointer, non-target composites that have
pointer components. However, it is more conservative than necessary, and
some existing tests expect its current results when less conservative
results seem reasonable.

This PR splits the logic into two cases:

1. Source values are the same: Return MayAlias when one value is the
address of a composite, and the other value is statically the address of
a pointer component of that composite.
2. Source values are different: Return MayAlias when one value is the
address of a composite (actual argument), and the other value is the
address of a pointer (dummy arg) that might dynamically be a component
of that composite.

In both cases, the actual implementation is still more conservative than
described above, but it can be improved further later. Details appear in
the comments.

Additionally, this PR revises the logic that reports MayAlias for a
pointer/target vs. another pointer/target. It constrains the existing
logic to handle only isData cases, and it adds less conservative
handling of !isData cases elsewhere. First, it extends case 2 listed
above to cover the case where the actual argument is the address of a
pointer rather than a composite. Second, it adds a third case: where
target attributes enable aliasing with a dummy argument.

Author: jdenny-ornl

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

@@ -157,6 +157,26 @@ struct AliasAnalysis {
     bool isData() const;
     bool isBoxData() const;
 
+    /// @name Dummy Argument Aliasing
+    ///
+    /// Check conditions related to dummy argument aliasing.
+    ///
+    /// For all uses, a result of false can prevent MayAlias from being
+    /// reported, so the list of cases where false is returned is conservative.
+
+    ///@{
+    /// The address of a (possibly host associated) dummy argument of the
+    /// current function?
+    bool mayBeDummyArgOrHostAssoc() const;
+    /// \c mayBeDummyArgOrHostAssoc and the address of a pointer?
+    bool mayBePtrDummyArgOrHostAssoc() const;
+    /// The address of an actual argument of the current function?
+    bool mayBeActualArg() const;
+    /// \c mayBeActualArg and the address of either a pointer or a composite
+    /// with a pointer component?
+    bool mayBeActualArgWithPtr(const mlir::Value *val) const;
+    ///@}
+
     mlir::Type getType() const;
   };
 

flang/lib/Optimizer/Analysis/AliasAnalysis.cpp

@@ -96,6 +96,39 @@ bool AliasAnalysis::Source::isBoxData() const {
          origin.isData;
 }
 
+bool AliasAnalysis::Source::mayBeDummyArgOrHostAssoc() const {
+  return kind != SourceKind::Allocate && kind != SourceKind::Global;
+}
+
+bool AliasAnalysis::Source::mayBePtrDummyArgOrHostAssoc() const {
+  // Must alias like dummy arg (or HostAssoc).
+  if (!mayBeDummyArgOrHostAssoc())
+    return false;
+  // Must be address of the dummy arg not of a dummy arg component.
+  if (isRecordWithPointerComponent(valueType))
+    return false;
+  // Must be address *of* (not *in*) a pointer.
+  return attributes.test(Attribute::Pointer) && !isData();
+}
+
+bool AliasAnalysis::Source::mayBeActualArg() const {
+  return kind != SourceKind::Allocate;
+}
+
+bool AliasAnalysis::Source::mayBeActualArgWithPtr(
+    const mlir::Value *val) const {
+  // Must not be local.
+  if (!mayBeActualArg())
+    return false;
+  // Can be address *of* (not *in*) a pointer.
+  if (attributes.test(Attribute::Pointer) && !isData())
+    return true;
+  // Can be address of a composite with a pointer component.
+  if (isRecordWithPointerComponent(val->getType()))
+    return true;
+  return false;
+}
+
 AliasResult AliasAnalysis::alias(mlir::Value lhs, mlir::Value rhs) {
   // TODO: alias() has to be aware of the function scopes.
   // After MLIR inlining, the current implementation may
@@ -118,13 +151,42 @@ AliasResult AliasAnalysis::alias(mlir::Value lhs, mlir::Value rhs) {
   }
 
   if (lhsSrc.kind == rhsSrc.kind) {
+    // If the kinds and origins are the same, then lhs and rhs must alias unless
+    // either source is approximate.  Approximate sources are for parts of the
+    // origin, but we don't have info here on which parts and whether they
+    // overlap, so we normally return MayAlias in that case.
     if (lhsSrc.origin == rhsSrc.origin) {
       LLVM_DEBUG(llvm::dbgs()
                  << "  aliasing because same source kind and origin\n");
       if (approximateSource)
         return AliasResult::MayAlias;
       return AliasResult::MustAlias;
     }
+    // If one value is the address of a composite, and if the other value is the
+    // address of a pointer/allocatable component of that composite, their
+    // origins compare unequal because the latter has !isData().  As for the
+    // address of any component vs. the address of the composite, a store to one
+    // can affect a load from the other, so the result should be MayAlias.  To
+    // catch this case, we conservatively return MayAlias when one value is the
+    // address of a composite, the other value is non-data, and they have the
+    // same origin value.
+    //
+    // TODO: That logic does not check that the latter is actually a component
+    // of the former, so it can return MayAlias when unnecessary.  For example,
+    // they might both be addresses of components of a larger composite.
+    //
+    // FIXME: Actually, we should generalize from isRecordWithPointerComponent
+    // to any composite because a component with !isData() is not always a
+    // pointer.  However, Source::isRecordWithPointerComponent currently doesn't
+    // actually check for pointer components, so it's fine for now.
+    if (lhsSrc.origin.u == rhsSrc.origin.u &&
+        ((isRecordWithPointerComponent(lhs.getType()) && !rhsSrc.isData()) ||
+         (isRecordWithPointerComponent(rhs.getType()) && !lhsSrc.isData()))) {
+      LLVM_DEBUG(llvm::dbgs()
+                 << "  aliasing between composite and non-data component with "
+                 << "same source kind and origin value\n");
+      return AliasResult::MayAlias;
+    }
 
     // Two host associated accesses may overlap due to an equivalence.
     if (lhsSrc.kind == SourceKind::HostAssoc) {
@@ -134,12 +196,17 @@ AliasResult AliasAnalysis::alias(mlir::Value lhs, mlir::Value rhs) {
   }
 
   Source *src1, *src2;
+  mlir::Value *val1, *val2;
   if (lhsSrc.kind < rhsSrc.kind) {
     src1 = &lhsSrc;
     src2 = &rhsSrc;
+    val1 = &lhs;
+    val2 = &rhs;
   } else {
     src1 = &rhsSrc;
     src2 = &lhsSrc;
+    val1 = &rhs;
+    val2 = &lhs;
   }
 
   if (src1->kind == SourceKind::Argument &&
@@ -162,23 +229,88 @@ AliasResult AliasAnalysis::alias(mlir::Value lhs, mlir::Value rhs) {
     src2->attributes.set(Attribute::Target);
   }
 
-  // Dummy TARGET/POINTER argument may alias with a global TARGET/POINTER.
+  // Two TARGET/POINTERs may alias.  The logic here focuses on data.  Handling
+  // of non-data is included below.
   if (src1->isTargetOrPointer() && src2->isTargetOrPointer() &&
-      src1->isData() == src2->isData()) {
+      src1->isData() && src2->isData()) {
     LLVM_DEBUG(llvm::dbgs() << "  aliasing because of target or pointer\n");
     return AliasResult::MayAlias;
   }
 
-  // Box for POINTER component inside an object of a derived type
-  // may alias box of a POINTER object, as well as boxes for POINTER
-  // components inside two objects of derived types may alias.
-  if ((isRecordWithPointerComponent(src1->valueType) &&
-       src2->isTargetOrPointer()) ||
-      (isRecordWithPointerComponent(src2->valueType) &&
-       src1->isTargetOrPointer()) ||
-      (isRecordWithPointerComponent(src1->valueType) &&
-       isRecordWithPointerComponent(src2->valueType))) {
-    LLVM_DEBUG(llvm::dbgs() << "  aliasing because of pointer components\n");
+  // Aliasing for dummy arg with target attribute.
+  //
+  // The address of a dummy arg (or HostAssoc) may alias the address of a
+  // non-local (global or another dummy arg) when both have target attributes.
+  // If either is a composite, addresses of components may alias as well.
+  //
+  // The previous "if" calling isTargetOrPointer casts a very wide net and so
+  // reports MayAlias for many such cases that would otherwise be reported here.
+  // It specifically skips such cases where one or both values have !isData()
+  // (e.g., address *of* pointer/allocatable component vs. address of
+  // composite), so this "if" catches those cases.
+  if (src1->attributes.test(Attribute::Target) &&
+      src2->attributes.test(Attribute::Target) &&
+      ((src1->mayBeDummyArgOrHostAssoc() && src2->mayBeActualArg()) ||
+       (src2->mayBeDummyArgOrHostAssoc() && src1->mayBeActualArg()))) {
+    LLVM_DEBUG(llvm::dbgs()
+               << "  aliasing between targets where one is a dummy arg\n");
+    return AliasResult::MayAlias;
+  }
+
+  // Aliasing for dummy arg that is a pointer.
+  //
+  // The address of a pointer dummy arg (but not a pointer component of a dummy
+  // arg) may alias the address of either (1) a non-local pointer or (2) thus a
+  // non-local composite with a pointer component.  A non-local might be a
+  // global or another dummy arg.  The following is an example of the global
+  // composite case:
+  //
+  // module m
+  //   type t
+  //      real, pointer :: p
+  //   end type
+  //   type(t) :: a
+  //   type(t) :: b
+  // contains
+  //   subroutine test(p)
+  //     real, pointer :: p
+  //     p = 42
+  //     a = b
+  //     print *, p
+  //   end subroutine
+  // end module
+  // program main
+  //   use m
+  //   real, target :: x1 = 1
+  //   real, target :: x2 = 2
+  //   a%p => x1
+  //   b%p => x2
+  //   call test(a%p)
+  // end
+  //
+  // The dummy argument p is an alias for a%p, even for the purposes of pointer
+  // association during the assignment a = b.  Thus, the program should print 2.
+  //
+  // The same is true when p is HostAssoc.  For example, we might replace the
+  // test subroutine above with:
+  //
+  // subroutine test(p)
+  //   real, pointer :: p
+  //   call internal()
+  // contains
+  //   subroutine internal()
+  //     p = 42
+  //     a = b
+  //     print *, p
+  //   end subroutine
+  // end subroutine
+  if ((src1->mayBePtrDummyArgOrHostAssoc() &&
+       src2->mayBeActualArgWithPtr(val2)) ||
+      (src2->mayBePtrDummyArgOrHostAssoc() &&
+       src1->mayBeActualArgWithPtr(val1))) {
+    LLVM_DEBUG(llvm::dbgs()
+               << "  aliasing between pointer dummy arg and either pointer or "
+               << "composite with pointer component\n");
     return AliasResult::MayAlias;
   }
 
@@ -278,6 +410,8 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
             followBoxData = true;
         })
         .Case<fir::ArrayCoorOp, fir::CoordinateOp>([&](auto op) {
+          if (isPointerReference(ty))
+            attributes.set(Attribute::Pointer);
           v = op->getOperand(0);
           defOp = v.getDefiningOp();
           if (mlir::isa<fir::BaseBoxType>(v.getType()))
@@ -396,6 +530,8 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
           defOp = v.getDefiningOp();
         })
         .Case<hlfir::DesignateOp>([&](auto op) {
+          auto varIf = llvm::cast<fir::FortranVariableOpInterface>(defOp);
+          attributes |= getAttrsFromVariable(varIf);
           // Track further through the memory indexed into
           // => if the source arrays/structures don't alias then nor do the
           //    results of hlfir.designate

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

@@ -20,8 +20,8 @@
 // CHECK-LABEL: Testing : "_QMmPtest
 // CHECK: a#0 <-> func.region0#0: MayAlias
 
-// FIXME: a's box cannot alias with raw reference to f32 (x), so MayAlias below must be NoAlias:
-// CHECK: a#0 <-> func.region0#1: MayAlias
+// a's box cannot alias with raw reference to f32 (x)
+// CHECK: a#0 <-> func.region0#1: NoAlias
 
 // pointer_dummy's box cannot alias with raw reference to f32 (x)
 // CHECK: func.region0#0 <-> func.region0#1: NoAlias
@@ -46,7 +46,7 @@ func.func private @_QPtest2(!fir.ref<f32>)
 
 // -----
 
-// A composite with a pointer component may alias with a dummy
+// A composite with a pointer component does not alias with a dummy
 // argument of composite type with a pointer component:
 // module m
 //   type t
@@ -63,7 +63,7 @@ func.func private @_QPtest2(!fir.ref<f32>)
 // end module m
 
 // CHECK-LABEL: Testing : "_QMmPtest"
-// CHECK: a#0 <-> func.region0#0: MayAlias
+// CHECK: a#0 <-> func.region0#0: NoAlias
 
 fir.global @_QMmEa : !fir.type<_QMmTt{pointer_component:!fir.box<!fir.ptr<f32>>}> {
   %0 = fir.undefined !fir.type<_QMmTt{pointer_component:!fir.box<!fir.ptr<f32>>}>
@@ -88,7 +88,7 @@ func.func private @_QPtest2(!fir.ref<f32>)
 // -----
 
 // Two dummy arguments of composite type with a pointer component
-// may alias each other:
+// do not alias each other:
 // module m
 //   type t
 //      real, pointer :: pointer_component
@@ -103,7 +103,7 @@ func.func private @_QPtest2(!fir.ref<f32>)
 // end module m
 
 // CHECK-LABEL: Testing : "_QMmPtest"
-// CHECK: func.region0#0 <-> func.region0#1: MayAlias
+// CHECK: func.region0#0 <-> func.region0#1: NoAlias
 
 func.func @_QMmPtest(%arg0: !fir.ref<!fir.type<_QMmTt{pointer_component:!fir.box<!fir.ptr<f32>>}>> {fir.bindc_name = "a"}, %arg1: !fir.ref<!fir.type<_QMmTt{pointer_component:!fir.box<!fir.ptr<f32>>}>> {fir.bindc_name = "b"}, %arg2: !fir.ref<f32> {fir.bindc_name = "x", fir.target}) attributes {test.ptr = "func"} {
   %0 = fir.field_index pointer_component, !fir.type<_QMmTt{pointer_component:!fir.box<!fir.ptr<f32>>}>
@@ -137,8 +137,7 @@ func.func private @_QPtest2(!fir.ref<f32>)
 // end module m
 
 // CHECK-LABEL: Testing : "_QMmPtest"
-// FIXME: MayAlias must be NoAlias
-// CHECK: func.region0#0 <-> func.region0#1: MayAlias
+// CHECK: func.region0#0 <-> func.region0#1: NoAlias
 
 func.func @_QMmPtest(%arg0: !fir.ref<!fir.type<_QMmTt{allocatable_component:!fir.box<!fir.heap<f32>>}>> {fir.bindc_name = "a"}, %arg1: !fir.ref<!fir.type<_QMmTt{allocatable_component:!fir.box<!fir.heap<f32>>}>> {fir.bindc_name = "b"}) attributes {test.ptr = "func"} {
   %0 = fir.field_index allocatable_component, !fir.type<_QMmTt{allocatable_component:!fir.box<!fir.heap<f32>>}>