[flang] AliasAnalysis: Fix pointer component logic

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

@@ -142,17 +142,16 @@ struct AliasAnalysis {
     /// Return true, if Target or Pointer attribute is set.
     bool isTargetOrPointer() const;
 
-    /// Return true, if the memory source's `valueType` is a reference type
-    /// to an object of derived type that contains a component with POINTER
-    /// attribute.
-    bool isRecordWithPointerComponent() const;
-
     bool isDummyArgument() const;
     bool isData() const;
     bool isBoxData() const;
 
     mlir::Type getType() const;
 
+    /// Return true, if `ty` is a reference type to an object of derived type
+    /// that contains a component with POINTER attribute.
+    static bool isRecordWithPointerComponent(mlir::Type ty);
+
     /// Return true, if `ty` is a reference type to a boxed
     /// POINTER object or a raw fir::PointerType.
     static bool isPointerReference(mlir::Type ty);

flang/lib/Optimizer/Analysis/AliasAnalysis.cpp

@@ -70,6 +70,14 @@ void AliasAnalysis::Source::print(llvm::raw_ostream &os) const {
   attributes.Dump(os, EnumToString);
 }
 
+bool AliasAnalysis::Source::isRecordWithPointerComponent(mlir::Type ty) {
+  auto eleTy = fir::dyn_cast_ptrEleTy(ty);
+  if (!eleTy)
+    return false;
+  // TO DO: Look for pointer components
+  return mlir::isa<fir::RecordType>(eleTy);
+}
+
 bool AliasAnalysis::Source::isPointerReference(mlir::Type ty) {
   auto eleTy = fir::dyn_cast_ptrEleTy(ty);
   if (!eleTy)
@@ -96,14 +104,6 @@ bool AliasAnalysis::Source::isBoxData() const {
          origin.isData;
 }
 
-bool AliasAnalysis::Source::isRecordWithPointerComponent() const {
-  auto eleTy = fir::dyn_cast_ptrEleTy(valueType);
-  if (!eleTy)
-    return false;
-  // TO DO: Look for pointer components
-  return mlir::isa<fir::RecordType>(eleTy);
-}
-
 AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
   auto lhsSrc = getSource(lhs);
   auto rhsSrc = getSource(rhs);
@@ -122,13 +122,45 @@ AliasResult AliasAnalysis::alias(Value lhs, 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
+    // Source::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 &&
+        ((Source::isRecordWithPointerComponent(lhs.getType()) &&
+          !rhsSrc.isData()) ||
+         (Source::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) {
@@ -138,12 +170,17 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
   }
 
   Source *src1, *src2;
+  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 &&
@@ -166,21 +203,56 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
     src2->attributes.set(Attribute::Target);
   }
 
-  // Dummy TARGET/POINTER argument may alias with a global TARGET/POINTER.
+  // Two TARGET/POINTERs may alias.
   if (src1->isTargetOrPointer() && src2->isTargetOrPointer() &&
       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 ((src1->isRecordWithPointerComponent() && src2->isTargetOrPointer()) ||
-      (src2->isRecordWithPointerComponent() && src1->isTargetOrPointer()) ||
-      (src1->isRecordWithPointerComponent() &&
-       src2->isRecordWithPointerComponent())) {
-    LLVM_DEBUG(llvm::dbgs() << "  aliasing because of pointer components\n");
+  // A pointer dummy arg (but not a pointer component of a dummy arg) may alias
+  // a pointer component and thus the associated composite.  That composite
+  // might be a global or another dummy arg.  This 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
+  //   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.
+  if ((Source::isRecordWithPointerComponent(val1->getType()) &&
+       src1->kind != SourceKind::Allocate &&
+       src2->kind == SourceKind::Argument &&
+       src2->attributes.test(Attribute::Pointer) && !src2->isData() &&
+       !Source::isRecordWithPointerComponent(src2->valueType)) ||
+      (Source::isRecordWithPointerComponent(val2->getType()) &&
+       src2->kind != SourceKind::Allocate &&
+       src1->kind == SourceKind::Argument &&
+       src1->attributes.test(Attribute::Pointer) && !src1->isData() &&
+       !Source::isRecordWithPointerComponent(src1->valueType))) {
+    LLVM_DEBUG(llvm::dbgs()
+               << "  aliasing between pointer arg and composite with pointer "
+               << "component\n");
     return AliasResult::MayAlias;
   }
 
@@ -350,6 +422,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>>}>