[flang] AliasAnalysis: Fix pointer component logic

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

@@ -157,6 +157,17 @@ struct AliasAnalysis {
     bool isData() const;
     bool isBoxData() const;
 
+    /// 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.
+    /// @{
+    bool aliasesLikeDummyArg() const;
+    bool aliasesLikePtrDummyArg() const;
+    bool canBeActualArg() const;
+    bool canBeActualArgWithPtr(const mlir::Value *val) const;
+    /// @}
+
     mlir::Type getType() const;
   };
 

flang/lib/Optimizer/Analysis/AliasAnalysis.cpp

@@ -94,6 +94,39 @@ bool AliasAnalysis::Source::isBoxData() const {
          origin.isData;
 }
 
+bool AliasAnalysis::Source::aliasesLikeDummyArg() const {
+  return kind != SourceKind::Allocate && kind != SourceKind::Global;
+}
+
+bool AliasAnalysis::Source::aliasesLikePtrDummyArg() const {
+  // Must alias like dummy arg (or HostAssoc).
+  if (!aliasesLikeDummyArg())
+    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::canBeActualArg() const {
+  return kind != SourceKind::Allocate;
+}
+
+bool AliasAnalysis::Source::canBeActualArgWithPtr(
+    const mlir::Value *val) const {
+  // Must not be local.
+  if (!canBeActualArg())
+    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
@@ -116,13 +149,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) {
@@ -132,12 +194,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 &&
@@ -160,23 +227,86 @@ 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->aliasesLikeDummyArg() && src2->canBeActualArg()) ||
+       (src2->aliasesLikeDummyArg() && src1->canBeActualArg()))) {
+    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->aliasesLikePtrDummyArg() && src2->canBeActualArgWithPtr(val2)) ||
+      (src2->aliasesLikePtrDummyArg() && src1->canBeActualArgWithPtr(val1))) {
+    LLVM_DEBUG(llvm::dbgs()
+               << "  aliasing between pointer dummy arg and either pointer or "
+               << "composite with pointer component\n");
     return AliasResult::MayAlias;
   }
 
@@ -272,6 +402,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()))
@@ -363,6 +495,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>>}>