[flang-commits] [flang] [flang] AliasAnalysis: More formally define and distinguish between data and non-data (PR #91020)

Renaud Kauffmann via flang-commits flang-commits at lists.llvm.org
Wed May 15 15:40:53 PDT 2024


https://github.com/Renaud-K updated https://github.com/llvm/llvm-project/pull/91020

>From 1cd675b8759000f42d15edaaa63da02b50d946c0 Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Fri, 3 May 2024 12:46:49 -0700
Subject: [PATCH 1/7] Remove SourceKind::Direct. Include data/non-data flang in
 source origin

---
 .../flang/Optimizer/Analysis/AliasAnalysis.h  |  31 ++++-
 .../lib/Optimizer/Analysis/AliasAnalysis.cpp  | 113 ++++++++----------
 .../lib/Optimizer/Transforms/AddAliasTags.cpp |  15 ++-
 .../AliasAnalysis/alias-analysis-2.fir        |  37 ++++--
 .../AliasAnalysis/alias-analysis-3.fir        |   4 +-
 5 files changed, 109 insertions(+), 91 deletions(-)

diff --git a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
index dfcafe88fee1b..b11ba229cf07e 100644
--- a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
+++ b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
@@ -38,10 +38,6 @@ struct AliasAnalysis {
              HostAssoc,
              /// Represents direct memory access whose source cannot be further
              /// determined
-             Direct,
-             /// Represents memory allocated by unknown means and
-             /// with the memory address defined by a memory reading
-             /// operation (e.g. fir::LoadOp).
              Indirect,
              /// Starting point to the analysis whereby nothing is known about
              /// the source
@@ -53,9 +49,17 @@ struct AliasAnalysis {
   struct Source {
     using SourceUnion = llvm::PointerUnion<mlir::SymbolRefAttr, mlir::Value>;
     using Attributes = Fortran::common::EnumSet<Attribute, Attribute_enumSize>;
+    
+    struct SourceOrigin {
+      /// Source definition of a value.
+      SourceUnion u;
+
+      /// Whether the source was reached following data or box reference
+      bool isData{false};
+    };
+
+    SourceOrigin origin;
 
-    /// Source definition of a value.
-    SourceUnion u;
     /// Kind of the memory source.
     SourceKind kind;
     /// Value type of the source definition.
@@ -77,6 +81,12 @@ struct AliasAnalysis {
     /// 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 a boxed
     /// POINTER object or a raw fir::PointerType.
     static bool isPointerReference(mlir::Type ty);
@@ -95,6 +105,15 @@ struct AliasAnalysis {
   Source getSource(mlir::Value);
 };
 
+inline bool operator==(const AliasAnalysis::Source::SourceOrigin &lhs,
+                       const AliasAnalysis::Source::SourceOrigin &rhs) {
+  return lhs.u == rhs.u && lhs.isData == rhs.isData;
+}
+inline bool operator!=(const AliasAnalysis::Source::SourceOrigin &lhs,
+                       const AliasAnalysis::Source::SourceOrigin &rhs) {
+  return !(lhs == rhs);
+}
+
 inline llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
                                      const AliasAnalysis::Source &op) {
   op.print(os);
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index ed1101dc5e8d8..4060ae649730b 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -33,11 +33,9 @@ static bool isDummyArgument(mlir::Value v) {
   if (!blockArg)
     return false;
 
-  mlir::Block *owner = blockArg.getOwner();
-  if (!owner->isEntryBlock() ||
-      !mlir::isa<mlir::FunctionOpInterface>(owner->getParentOp()))
-    return false;
-  return true;
+  auto *owner{blockArg.getOwner()};
+  return owner->isEntryBlock() &&
+         mlir::isa<mlir::FunctionOpInterface>(owner->getParentOp());
 }
 
 /// Temporary function to skip through all the no op operations
@@ -58,12 +56,17 @@ static mlir::Value getOriginalDef(mlir::Value v) {
 namespace fir {
 
 void AliasAnalysis::Source::print(llvm::raw_ostream &os) const {
-  if (auto v = llvm::dyn_cast<mlir::Value>(u))
+  if (auto v = llvm::dyn_cast<mlir::Value>(origin.u))
     os << v;
-  else if (auto gbl = llvm::dyn_cast<mlir::SymbolRefAttr>(u))
+  else if (auto gbl = llvm::dyn_cast<mlir::SymbolRefAttr>(origin.u))
     os << gbl;
   os << " SourceKind: " << EnumToString(kind);
   os << " Type: " << valueType << " ";
+  if (origin.isData) {
+    os << " following data ";
+  } else {
+    os << " following box reference ";
+  }
   attributes.Dump(os, EnumToString);
 }
 
@@ -80,6 +83,19 @@ bool AliasAnalysis::Source::isTargetOrPointer() const {
          attributes.test(Attribute::Target);
 }
 
+bool AliasAnalysis::Source::isDummyArgument() const {
+  if (auto v = origin.u.dyn_cast<mlir::Value>()) {
+    return ::isDummyArgument(v);
+  }
+  return false;
+}
+
+bool AliasAnalysis::Source::isData() const { return origin.isData; }
+bool AliasAnalysis::Source::isBoxData() const {
+  return mlir::isa<fir::BaseBoxType>(fir::unwrapRefType(valueType)) &&
+         origin.isData;
+}
+
 bool AliasAnalysis::Source::isRecordWithPointerComponent() const {
   auto eleTy = fir::dyn_cast_ptrEleTy(valueType);
   if (!eleTy)
@@ -101,29 +117,20 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
 
   // Indirect case currently not handled. Conservatively assume
   // it aliases with everything
-  if (lhsSrc.kind > SourceKind::Direct || rhsSrc.kind > SourceKind::Direct) {
+  if (lhsSrc.kind >= SourceKind::Indirect ||
+      rhsSrc.kind >= SourceKind::Indirect) {
     return AliasResult::MayAlias;
   }
 
-  // SourceKind::Direct is set for the addresses wrapped in a global boxes.
-  // ie: fir.global @_QMpointersEp : !fir.box<!fir.ptr<f32>>
-  // Though nothing is known about them, they would only alias with targets or
-  // pointers
-  bool directSourceToNonTargetOrPointer = false;
-  if (lhsSrc.u != rhsSrc.u || lhsSrc.kind != rhsSrc.kind) {
-    if ((lhsSrc.kind == SourceKind::Direct && !rhsSrc.isTargetOrPointer()) ||
-        (rhsSrc.kind == SourceKind::Direct && !lhsSrc.isTargetOrPointer()))
-      directSourceToNonTargetOrPointer = true;
-  }
-
-  if (lhsSrc.kind == SourceKind::Direct ||
-      rhsSrc.kind == SourceKind::Direct) {
-    if (!directSourceToNonTargetOrPointer)
-      return AliasResult::MayAlias;
+  // If we have reached the same source but comparing box reference against
+  // data we are not comparing apples-to-apples. The 2 cannot alias.
+  if ((lhsSrc.origin.u == rhsSrc.origin.u) &&
+      lhsSrc.isData() != rhsSrc.isData()) {
+    return AliasResult::NoAlias;
   }
 
   if (lhsSrc.kind == rhsSrc.kind) {
-    if (lhsSrc.u == rhsSrc.u) {
+    if (lhsSrc.origin == rhsSrc.origin) {
       if (approximateSource)
         return AliasResult::MayAlias;
       return AliasResult::MustAlias;
@@ -133,13 +140,9 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
     if (lhsSrc.kind == SourceKind::HostAssoc)
       return AliasResult::MayAlias;
 
-    // Allocate and global memory address cannot physically alias
-    if (lhsSrc.kind == SourceKind::Allocate ||
-        lhsSrc.kind == SourceKind::Global)
-      return AliasResult::NoAlias;
-
-    // Dummy TARGET/POINTER arguments may alias.
-    if (lhsSrc.isTargetOrPointer() && rhsSrc.isTargetOrPointer())
+    // TARGET/POINTER arguments may alias.
+    if (lhsSrc.isTargetOrPointer() && rhsSrc.isTargetOrPointer() &&
+        lhsSrc.isData() == rhsSrc.isData())
       return AliasResult::MayAlias;
 
     // Box for POINTER component inside an object of a derived type
@@ -186,7 +189,8 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
   }
 
   // Dummy TARGET/POINTER argument may alias with a global TARGET/POINTER.
-  if (src1->isTargetOrPointer() && src2->isTargetOrPointer())
+  if (src1->isTargetOrPointer() && src2->isTargetOrPointer() &&
+      src1->isData() == src2->isData())
     return AliasResult::MayAlias;
 
   // Box for POINTER component inside an object of a derived type
@@ -262,7 +266,10 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
   mlir::Type ty;
   bool breakFromLoop{false};
   bool approximateSource{false};
-  bool followBoxAddr{mlir::isa<fir::BaseBoxType>(v.getType())};
+  bool followBoxData{mlir::isa<fir::BaseBoxType>(v.getType())};
+  bool isBoxRef{fir::isa_ref_type(v.getType()) &&
+                mlir::isa<fir::BaseBoxType>(fir::unwrapRefType(v.getType()))};
+  bool followingData = !isBoxRef || followBoxData;
   mlir::SymbolRefAttr global;
   Source::Attributes attributes;
   while (defOp && !breakFromLoop) {
@@ -282,24 +289,24 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
           v = op->getOperand(0);
           defOp = v.getDefiningOp();
           if (mlir::isa<fir::BaseBoxType>(v.getType()))
-            followBoxAddr = true;
+            followBoxData = true;
         })
         .Case<fir::ArrayCoorOp, fir::CoordinateOp>([&](auto op) {
           v = op->getOperand(0);
           defOp = v.getDefiningOp();
           if (mlir::isa<fir::BaseBoxType>(v.getType()))
-            followBoxAddr = true;
+            followBoxData = true;
           approximateSource = true;
         })
         .Case<fir::EmboxOp, fir::ReboxOp>([&](auto op) {
-          if (followBoxAddr) {
+          if (followBoxData) {
             v = op->getOperand(0);
             defOp = v.getDefiningOp();
           } else
             breakFromLoop = true;
         })
         .Case<fir::LoadOp>([&](auto op) {
-          if (followBoxAddr && mlir::isa<fir::BaseBoxType>(op.getType())) {
+          if (followBoxData && mlir::isa<fir::BaseBoxType>(op.getType())) {
             // For now, support the load of an argument or fir.address_of
             // TODO: generalize to all operations (in particular fir.alloca and
             // fir.allocmem)
@@ -318,24 +325,7 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
         .Case<fir::AddrOfOp>([&](auto op) {
           // Address of a global scope object.
           ty = v.getType();
-
-          // When the global is a
-          // fir.global @_QMpointersEp : !fir.box<!fir.ptr<f32>>
-          //   or
-          // fir.global @_QMpointersEp : !fir.box<!fir.heap<f32>>
-          //
-          // and when following through the wrapped address, capture
-          // the fact that there is nothing known about it. Therefore setting
-          // the source to Direct.
-          //
-          // When not following the wrapped address, then consider the address
-          // of the box, which has nothing to do with the wrapped address and
-          // lies in the global memory space.
-          if (followBoxAddr &&
-              mlir::isa<fir::BaseBoxType>(fir::unwrapRefType(ty)))
-            type = SourceKind::Direct;
-          else
-            type = SourceKind::Global;
+          type = SourceKind::Global;
 
           auto globalOpName = mlir::OperationName(
               fir::GlobalOp::getOperationName(), defOp->getContext());
@@ -343,11 +333,10 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
                   v, fir::GlobalOp::getTargetAttrName(globalOpName)))
             attributes.set(Attribute::Target);
 
-          // TODO: Take followBoxAddr into account when setting the pointer
+          // TODO: Take followBoxData into account when setting the pointer
           // attribute
           if (Source::isPointerReference(ty))
             attributes.set(Attribute::Pointer);
-
           global = llvm::cast<fir::AddrOfOp>(op).getSymbol();
           breakFromLoop = true;
         })
@@ -393,7 +382,7 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
           // MustAlias after going through a designate operation
           approximateSource = true;
           if (mlir::isa<fir::BaseBoxType>(v.getType()))
-            followBoxAddr = true;
+            followBoxData = true;
         })
         .Default([&](auto op) {
           defOp = nullptr;
@@ -412,10 +401,10 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
         attributes.set(Attribute::Pointer);
     }
 
-  if (type == SourceKind::Global || type == SourceKind::Direct)
-    return {global, type, ty, attributes, approximateSource};
-
-  return {v, type, ty, attributes, approximateSource};
+  if (type == SourceKind::Global) {
+    return {{global, followingData}, type, ty, attributes, approximateSource};
+  }
+  return {{v, followingData}, type, ty, attributes, approximateSource};
 }
 
 } // namespace fir
diff --git a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
index 3642a812096db..5be42e0f871cd 100644
--- a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
+++ b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
@@ -145,7 +145,7 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
       source.kind == fir::AliasAnalysis::SourceKind::Argument) {
     LLVM_DEBUG(llvm::dbgs().indent(2)
                << "Found reference to dummy argument at " << *op << "\n");
-    std::string name = getFuncArgName(source.u.get<mlir::Value>());
+    std::string name = getFuncArgName(source.origin.u.get<mlir::Value>());
     if (!name.empty())
       tag = state.getFuncTree(func).dummyArgDataTree.getTag(name);
     else
@@ -155,18 +155,17 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
 
     // TBAA for global variables
   } else if (enableGlobals &&
-             source.kind == fir::AliasAnalysis::SourceKind::Global) {
-    mlir::SymbolRefAttr glbl = source.u.get<mlir::SymbolRefAttr>();
+             source.kind == fir::AliasAnalysis::SourceKind::Global && !source.isBoxData()) {
+    mlir::SymbolRefAttr glbl = source.origin.u.get<mlir::SymbolRefAttr>();
     const char *name = glbl.getRootReference().data();
     LLVM_DEBUG(llvm::dbgs().indent(2) << "Found reference to global " << name
                                       << " at " << *op << "\n");
     tag = state.getFuncTree(func).globalDataTree.getTag(name);
 
     // TBAA for SourceKind::Direct
-  } else if (enableDirect &&
-             source.kind == fir::AliasAnalysis::SourceKind::Direct) {
-    if (source.u.is<mlir::SymbolRefAttr>()) {
-      mlir::SymbolRefAttr glbl = source.u.get<mlir::SymbolRefAttr>();
+  } else if (enableDirect && source.isBoxData()) {
+    if (source.origin.u.is<mlir::SymbolRefAttr>()) {
+      mlir::SymbolRefAttr glbl = source.origin.u.get<mlir::SymbolRefAttr>();
       const char *name = glbl.getRootReference().data();
       LLVM_DEBUG(llvm::dbgs().indent(2) << "Found reference to direct " << name
                                         << " at " << *op << "\n");
@@ -182,7 +181,7 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
   } else if (enableLocalAllocs &&
              source.kind == fir::AliasAnalysis::SourceKind::Allocate) {
     std::optional<llvm::StringRef> name;
-    mlir::Operation *sourceOp = source.u.get<mlir::Value>().getDefiningOp();
+    mlir::Operation *sourceOp = source.origin.u.get<mlir::Value>().getDefiningOp();
     if (auto alloc = mlir::dyn_cast_or_null<fir::AllocaOp>(sourceOp))
       name = alloc.getUniqName();
     else if (auto alloc = mlir::dyn_cast_or_null<fir::AllocMemOp>(sourceOp))
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
index 31459ef21d947..af3e491e062c3 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
@@ -37,14 +37,14 @@
 
 // arg2 is a reference to a pointer. Modifying arg2 could
 // modify a target with a pointer component
-// CHECK-DAG: func.region0#0 <-> func.region0#2: MayAlias
-// CHECK-DAG: func.region0#1 <-> func.region0#2: MayAlias
+// CHECK-DAG: arg2.load#0 <-> func.region0#0: MayAlias
+// CHECK-DAG: arg2.load#0 <-> func.region0#1: MayAlias
 
 // However, the address wrapped by arg2, can alias with any target or
 // pointer arguments
 // CHECK-DAG: arg2.addr#0 <-> func.region0#0: MayAlias
 // CHECK-DAG: arg2.addr#0 <-> func.region0#1: MayAlias
-// CHECK-DAG: arg2.addr#0 <-> func.region0#2: MustAlias
+// CHECK-DAG: arg2.load#0 <-> arg2.addr#0: MustAlias
 // CHECK-DAG: boxp1.addr#0 <-> arg2.addr#0: MayAlias
 
 func.func @_QFPtest(%arg0: !fir.ref<f32> {fir.bindc_name = "v1", fir.target}, %arg1: !fir.ref<f32> {fir.bindc_name = "v2", fir.target}, %arg2: !fir.ref<!fir.box<!fir.ptr<f32>>> ) attributes {test.ptr = "func"} {
@@ -80,7 +80,7 @@ func.func @_QFPtest(%arg0: !fir.ref<f32> {fir.bindc_name = "v1", fir.target}, %a
   %14 = fir.box_addr %13 : (!fir.box<!fir.ptr<f32>>) -> !fir.ptr<f32>
   fir.store %14 to %4 : !fir.ref<!fir.ptr<f32>>
 
-  %15 = fir.load %arg2 : !fir.ref<!fir.box<!fir.ptr<f32>>>
+  %15 = fir.load %arg2 {test.ptr = "arg2.load"} : !fir.ref<!fir.box<!fir.ptr<f32>>>
   %16 = fir.box_addr %15 {test.ptr = "arg2.addr"} : (!fir.box<!fir.ptr<f32>>) -> !fir.ptr<f32>
   return
 }
@@ -99,10 +99,12 @@ func.func @_QFPtest(%arg0: !fir.ref<f32> {fir.bindc_name = "v1", fir.target}, %a
 // CHECK-DAG: func.region0#1 <-> func.region0#2: MayAlias
 
 // They can also modify targets that have pointer components
-// CHECK-DAG: func.region0#0 <-> func.region0#1: MayAlias
-// CHECK-DAG: func.region0#0 <-> func.region0#2: MayAlias
+// CHECK-DAG: arg1.load#0 <-> func.region0#0: MayAlias
+// CHECK-DAG: arg2.load#0 <-> func.region0#0: MayAlias
 
 func.func @_QFPtest2(%arg0: !fir.ref<f32> {fir.bindc_name = "v1", fir.target}, %arg1: !fir.ref<!fir.box<!fir.ptr<f32>>>, %arg2: !fir.ref<!fir.box<!fir.ptr<f32>>> ) attributes {test.ptr = "func"} {
+  %0 = fir.load %arg1 {test.ptr = "arg1.load"} : !fir.ref<!fir.box<!fir.ptr<f32>>>
+  %1 = fir.load %arg2 {test.ptr = "arg2.load"} : !fir.ref<!fir.box<!fir.ptr<f32>>>
   return
 }
 
@@ -141,14 +143,14 @@ func.func @_QFPtest2(%arg0: !fir.ref<f32> {fir.bindc_name = "v1", fir.target}, %
 // alias with the wrapped scalar _QFEvar2. We meant box_addr of _QMpointersEp 
 // CHECK-DAG: p#0 <-> box.addr#0: NoAlias
 
-// TODO: Still need to handle more gracefully the difference between !fir.ref<!fir.box<>> and !fir.box<> 
-// CHECK-DAG: box.addr#0 <-> func.region0#0: MayAlias
+// Now handling more gracefully the difference between !fir.ref<!fir.box<>> and !fir.box<> 
+// CHECK-DAG: box.addr#0 <-> func.region0#0: NoAlias
 
 // var2, although it is a target, cannot alias with p
 // A modification of p would only make them point to a new target but not modify it
 // CHECK-DAG: var2#0 <-> p#0: NoAlias
 // It can alias with p1, if p1 is a pointer component
-// CHECK-DAG: var2#0 <-> func.region0#0: MayAlias
+// CHECK-DAG: arg0.load#0 <-> var2#0: MayAlias
 // It is the same as box.addr
 // CHECK-DAG: var2#0 <-> box.addr#0: MustAlias
 
@@ -173,6 +175,7 @@ fir.global internal @_QFEvar2 target : f32 {
 }
 
 func.func @_QFPtest3(%arg0: !fir.ref<!fir.box<!fir.ptr<f32>>> {fir.bindc_name = "p1"}, %arg1: !fir.ref<f32>) attributes {test.ptr = "func"} {
+  %3 = fir.load %arg0 {test.ptr = "arg0.load"}: !fir.ref<!fir.box<!fir.ptr<f32>>>
   %4 = fir.address_of(@_QFEvar2) {test.ptr = "var2"} : !fir.ref<f32>
   %5 = fir.address_of(@_QMpointersEp) {test.ptr = "p"} : !fir.ref<!fir.box<!fir.ptr<f32>>>
   %6 = fir.embox %4 : (!fir.ref<f32>) -> !fir.box<!fir.ptr<f32>>
@@ -214,10 +217,17 @@ func.func @_QFPtest3(%arg0: !fir.ref<!fir.box<!fir.ptr<f32>>> {fir.bindc_name =
 // CHECK-DAG: var2_hlfir#1 <-> p_hlfir#0: NoAlias
 // CHECK-DAG: var2_hlfir#1 <-> p_hlfir#1: NoAlias
 
-// CHECK-DAG: var2#0 <-> func.region0#0: MayAlias
-// CHECK-DAG: var2_fir#0 <-> func.region0#0: MayAlias
-// CHECK-DAG: var2_hlfir#0 <-> func.region0#0: MayAlias
-// CHECK-DAG: var2_hlfir#1 <-> func.region0#0: MayAlias
+// The data cannot alias with the box references
+// CHECK-DAG: var2#0 <-> func.region0#0: NoAlias
+// CHECK-DAG: var2_fir#0 <-> func.region0#0: NoAlias
+// CHECK-DAG: var2_hlfir#0 <-> func.region0#0: NoAlias
+// CHECK-DAG: var2_hlfir#1 <-> func.region0#0: NoAlias
+
+// But it can alias with the box's own data
+// CHECK-DAG: arg0.load#0 <-> var2#0: MayAlias
+// CHECK-DAG: arg0.load#0 <-> var2_fir#0: MayAlias
+// CHECK-DAG: arg0.load#0 <-> var2_hlfir#0: MayAlias
+// CHECK-DAG: arg0.load#0 <-> var2_hlfir#1: MayAlias
 
 // CHECK-DAG: var2#0 <-> box.addr#0: MustAlias
 // CHECK-DAG: var2#0 <-> box.addr_fir#0: MustAlias
@@ -255,6 +265,7 @@ fir.global internal @_QFEvar2 target : f32 {
 }
 
 func.func @_QFPtest4(%arg0: !fir.ref<!fir.box<!fir.ptr<f32>>> {fir.bindc_name = "p1"}, %arg1: !fir.ref<f32>) attributes {test.ptr = "func"} {
+  %3 = fir.load %arg0 {test.ptr = "arg0.load"} : !fir.ref<!fir.box<!fir.ptr<f32>>>
   %4 = fir.address_of(@_QFEvar2) {test.ptr = "var2"} : !fir.ref<f32>
   %fir_decl_var2 = fir.declare %4 {uniq_name = "var2_fir", test.ptr = "var2_fir"}: (!fir.ref<f32>) -> !fir.ref<f32>
   %hlfir_decl_var2:2 = hlfir.declare %4 {uniq_name = "var2_hlfir", test.ptr = "var2_hlfir"}: (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir
index d7bd970b51907..3d576cb5c0af9 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir
@@ -23,8 +23,8 @@
 // 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
 
-// FIXME: pointer_dummy's box cannot alias with raw reference to f32 (x), so MayAlias below must be NoAlias:
-// CHECK: func.region0#0 <-> func.region0#1: MayAlias
+// pointer_dummy's box cannot alias with raw reference to f32 (x), so MayAlias below must be NoAlias:
+// CHECK: func.region0#0 <-> func.region0#1: 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>>}>

>From 49c550b9c1391fea43cb0f455d15a0c086bec729 Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Fri, 3 May 2024 15:28:44 -0700
Subject: [PATCH 2/7] Applied formatting

---
 flang/include/flang/Optimizer/Analysis/AliasAnalysis.h | 2 +-
 flang/lib/Optimizer/Transforms/AddAliasTags.cpp        | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
index b11ba229cf07e..64835ace68955 100644
--- a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
+++ b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
@@ -49,7 +49,7 @@ struct AliasAnalysis {
   struct Source {
     using SourceUnion = llvm::PointerUnion<mlir::SymbolRefAttr, mlir::Value>;
     using Attributes = Fortran::common::EnumSet<Attribute, Attribute_enumSize>;
-    
+
     struct SourceOrigin {
       /// Source definition of a value.
       SourceUnion u;
diff --git a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
index 5be42e0f871cd..4f02382f39807 100644
--- a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
+++ b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
@@ -155,7 +155,8 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
 
     // TBAA for global variables
   } else if (enableGlobals &&
-             source.kind == fir::AliasAnalysis::SourceKind::Global && !source.isBoxData()) {
+             source.kind == fir::AliasAnalysis::SourceKind::Global &&
+             !source.isBoxData()) {
     mlir::SymbolRefAttr glbl = source.origin.u.get<mlir::SymbolRefAttr>();
     const char *name = glbl.getRootReference().data();
     LLVM_DEBUG(llvm::dbgs().indent(2) << "Found reference to global " << name
@@ -181,7 +182,8 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
   } else if (enableLocalAllocs &&
              source.kind == fir::AliasAnalysis::SourceKind::Allocate) {
     std::optional<llvm::StringRef> name;
-    mlir::Operation *sourceOp = source.origin.u.get<mlir::Value>().getDefiningOp();
+    mlir::Operation *sourceOp =
+        source.origin.u.get<mlir::Value>().getDefiningOp();
     if (auto alloc = mlir::dyn_cast_or_null<fir::AllocaOp>(sourceOp))
       name = alloc.getUniqName();
     else if (auto alloc = mlir::dyn_cast_or_null<fir::AllocMemOp>(sourceOp))

>From cf100932dd22433c044120c7790d81b57aa36357 Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Fri, 3 May 2024 17:05:02 -0700
Subject: [PATCH 3/7] Restoring previous behavior on enableDirect

---
 flang/lib/Optimizer/Transforms/AddAliasTags.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
index 4f02382f39807..caece93840043 100644
--- a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
+++ b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
@@ -164,7 +164,9 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
     tag = state.getFuncTree(func).globalDataTree.getTag(name);
 
     // TBAA for SourceKind::Direct
-  } else if (enableDirect && source.isBoxData()) {
+  } else if (enableDirect &&
+             source.kind == fir::AliasAnalysis::SourceKind::Global &&
+             source.isBoxData()) {
     if (source.origin.u.is<mlir::SymbolRefAttr>()) {
       mlir::SymbolRefAttr glbl = source.origin.u.get<mlir::SymbolRefAttr>();
       const char *name = glbl.getRootReference().data();

>From cea1244cac428d4072d74b2ff89eb81aa441eb2f Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Mon, 6 May 2024 17:06:52 -0700
Subject: [PATCH 4/7] Applying review feedback and additional clean-ups

---
 .../flang/Optimizer/Analysis/AliasAnalysis.h  | 55 +++++++++++++++-
 .../lib/Optimizer/Analysis/AliasAnalysis.cpp  | 64 +++++++------------
 .../AliasAnalysis/alias-analysis-9.fir        | 51 +++++++++++++++
 3 files changed, 126 insertions(+), 44 deletions(-)
 create mode 100644 flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir

diff --git a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
index 64835ace68955..8a0eb525f1915 100644
--- a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
+++ b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
@@ -36,8 +36,9 @@ struct AliasAnalysis {
              /// Represents memory allocated outside of a function
              /// and passed to the function via host association tuple.
              HostAssoc,
-             /// Represents direct memory access whose source cannot be further
-             /// determined
+             /// Represents memory allocated by unknown means and
+             /// with the memory address defined by a memory reading
+             /// operation (e.g. fir::LoadOp).
              Indirect,
              /// Starting point to the analysis whereby nothing is known about
              /// the source
@@ -46,6 +47,56 @@ struct AliasAnalysis {
   /// Attributes of the memory source object.
   ENUM_CLASS(Attribute, Target, Pointer, IntentIn);
 
+  // See
+  // https://discourse.llvm.org/t/rfc-distinguish-between-data-and-non-data-in-fir-alias-analysis/78759/1
+  //
+  // It is possible, while following the source of a memory reference through
+  // the use-def chain, to arrive at the same origin, even though the starting
+  // points were known to not alias.
+  //
+  // Example:
+  // clang-format off
+  //    fir.global @_QMtopEa : !fir.box<!fir.ptr<!fir.array<?xf32>>> 
+  //  
+  //  func.func @_QPtest() {
+  //    %c1 = arith.constant 1 : index
+  //    %cst = arith.constant 1.000000e+00 : f32
+  //    %0 = fir.address_of(@_QMtopEa) : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
+  //    %1 = fir.declare %0 {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QMtopEa"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>) -> !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
+  //    %2 = fir.load %1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
+  //    ...
+  //    %5 = fir.array_coor %2 %c1 : (!fir.box<!fir.ptr<!fir.array<?xf32>>>, !fir.shift<1>, index) -> !fir.ref<f32>
+  //    fir.store %cst to %5 : !fir.ref<f32>
+  //    return
+  //  }
+  //
+  // With high level operations, such as fir.array_coor, it is possible to
+  // reach into the data wrapped by the box (the descriptor) therefore when
+  // asking about the memory source of the %5, we are really asking about the
+  // source of the data of box %2.
+  //
+  // When asking about the source of %0 which is the address of the box, we
+  // reach the same source as in the first case: the global @_QMtopEa. Yet one
+  // source refers to the data while the other refers to the address of the box
+  // itself.
+  //
+  // To distinguish between the two, the isData flag has been added, whereby
+  // data is defined as any memory reference that is not a box reference.
+  // Additionally, because it is relied on in HLFIR lowering, we allow querying
+  // on a box SSA value, which is interpreted as querying on its data.
+  //
+  // So in the above example, !fir.ref<f32> and !fir.box<!fir.ptr<!fir.array<?xf32>>> is data, 
+  // while !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>> is not data.
+
+  // This generally applies to function arguments. In the example below, %arg0
+  // is data, %arg1 is not data but a load of %arg1 is.
+  // 
+  // func.func @_QFPtest2(%arg0: !fir.ref<f32>, %arg1: !fir.ref<!fir.box<!fir.ptr<f32>>> )  {
+  //    %0 = fir.load %arg1 : !fir.ref<!fir.box<!fir.ptr<f32>>>
+  //    ... }
+  //
+  // clang-format on
+
   struct Source {
     using SourceUnion = llvm::PointerUnion<mlir::SymbolRefAttr, mlir::Value>;
     using Attributes = Fortran::common::EnumSet<Attribute, Attribute_enumSize>;
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index 4060ae649730b..ade240bc2364e 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -108,12 +108,11 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
   auto lhsSrc = getSource(lhs);
   auto rhsSrc = getSource(rhs);
   bool approximateSource = lhsSrc.approximateSource || rhsSrc.approximateSource;
-  LLVM_DEBUG(llvm::dbgs() << "AliasAnalysis::alias\n";
+  LLVM_DEBUG(llvm::dbgs() << "\n"; llvm::dbgs() << "AliasAnalysis::alias\n";
              llvm::dbgs() << "  lhs: " << lhs << "\n";
              llvm::dbgs() << "  lhsSrc: " << lhsSrc << "\n";
              llvm::dbgs() << "  rhs: " << rhs << "\n";
-             llvm::dbgs() << "  rhsSrc: " << rhsSrc << "\n";
-             llvm::dbgs() << "\n";);
+             llvm::dbgs() << "  rhsSrc: " << rhsSrc << "\n";);
 
   // Indirect case currently not handled. Conservatively assume
   // it aliases with everything
@@ -122,43 +121,22 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
     return AliasResult::MayAlias;
   }
 
-  // If we have reached the same source but comparing box reference against
-  // data we are not comparing apples-to-apples. The 2 cannot alias.
-  if ((lhsSrc.origin.u == rhsSrc.origin.u) &&
-      lhsSrc.isData() != rhsSrc.isData()) {
-    return AliasResult::NoAlias;
-  }
-
   if (lhsSrc.kind == rhsSrc.kind) {
     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;
     }
 
     // Two host associated accesses may overlap due to an equivalence.
-    if (lhsSrc.kind == SourceKind::HostAssoc)
-      return AliasResult::MayAlias;
-
-    // TARGET/POINTER arguments may alias.
-    if (lhsSrc.isTargetOrPointer() && rhsSrc.isTargetOrPointer() &&
-        lhsSrc.isData() == rhsSrc.isData())
+    if (lhsSrc.kind == SourceKind::HostAssoc) {
+      LLVM_DEBUG(llvm::dbgs() << "  aliasing because of host association\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 ((lhsSrc.isRecordWithPointerComponent() && rhsSrc.isTargetOrPointer()) ||
-        (rhsSrc.isRecordWithPointerComponent() && lhsSrc.isTargetOrPointer()) ||
-        (lhsSrc.isRecordWithPointerComponent() &&
-         rhsSrc.isRecordWithPointerComponent()))
-      return AliasResult::MayAlias;
-
-    return AliasResult::NoAlias;
+    }
   }
 
-  assert(lhsSrc.kind != rhsSrc.kind && "memory source kinds must be different");
-
   Source *src1, *src2;
   if (lhsSrc.kind < rhsSrc.kind) {
     src1 = &lhsSrc;
@@ -190,8 +168,10 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
 
   // Dummy TARGET/POINTER argument may alias with a global TARGET/POINTER.
   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
@@ -199,8 +179,10 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
   if ((src1->isRecordWithPointerComponent() && src2->isTargetOrPointer()) ||
       (src2->isRecordWithPointerComponent() && src1->isTargetOrPointer()) ||
       (src1->isRecordWithPointerComponent() &&
-       src2->isRecordWithPointerComponent()))
+       src2->isRecordWithPointerComponent())) {
+    LLVM_DEBUG(llvm::dbgs() << "  aliasing because of pointer components\n");
     return AliasResult::MayAlias;
+  }
 
   return AliasResult::NoAlias;
 }
@@ -306,17 +288,15 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
             breakFromLoop = true;
         })
         .Case<fir::LoadOp>([&](auto op) {
-          if (followBoxData && mlir::isa<fir::BaseBoxType>(op.getType())) {
-            // For now, support the load of an argument or fir.address_of
-            // TODO: generalize to all operations (in particular fir.alloca and
-            // fir.allocmem)
-            auto def = getOriginalDef(op.getMemref());
-            if (isDummyArgument(def) ||
-                def.template getDefiningOp<fir::AddrOfOp>()) {
-              v = def;
-              defOp = v.getDefiningOp();
-              return;
-            }
+          // If the load is from a leaf source, return the leaf. Do not track
+          // through indirections otherwise.
+          // TODO: At support to fir.alloca and fir.allocmem
+          auto def = getOriginalDef(op.getMemref());
+          if (isDummyArgument(def) ||
+              def.template getDefiningOp<fir::AddrOfOp>()) {
+            v = def;
+            defOp = v.getDefiningOp();
+            return;
           }
           // No further tracking for addresses loaded from memory for now.
           type = SourceKind::Indirect;
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
new file mode 100644
index 0000000000000..2b92872a2c4b5
--- /dev/null
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
@@ -0,0 +1,51 @@
+// RUN: fir-opt %s -pass-pipeline='builtin.module(func.func(test-fir-alias-analysis))' 2>&1 | FileCheck %s
+
+
+// module m
+// type t
+//  type(t), pointer :: next
+//  integer :: i
+// end type
+// contains
+// subroutine foo(x, y)
+//   type(t) :: x, y
+//   integer :: i1, i2
+//   i1 = x%next%i
+//   x = y
+//   i2 = x%next%i
+// end subroutine
+// end module
+
+// CHECK-LABEL: Testing : "_QMmPfoo"
+// TODO: x and y do not alias
+// CHECK-DAG: x#0 <-> y#0: MayAlias
+// CHECK-DAG: y#0 <-> xnext1#0: MayAlias
+// CHECK-DAG: y#0 <-> xnext2#0: MayAlias
+
+// These however do alias
+// CHECK-DAG: x#0 <-> xnext1#0: MayAlias
+// CHECK-DAG: x#0 <-> xnext2#0: MayAlias
+// CHECK-DAG: xnext1#0 <-> xnext2#0: MayAlias
+
+func.func @_QMmPfoo(%arg0: !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>> {fir.bindc_name = "x"}, %arg1: !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>> {fir.bindc_name = "y"}) {
+  %0 = fir.alloca i32 {bindc_name = "i1", uniq_name = "_QMmFfooEi1"}
+  %1:2 = hlfir.declare %0 {uniq_name = "_QMmFfooEi1"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+  %2 = fir.alloca i32 {bindc_name = "i2", uniq_name = "_QMmFfooEi2"}
+  %3:2 = hlfir.declare %2 {uniq_name = "_QMmFfooEi2"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+  %4:2 = hlfir.declare %arg0 {uniq_name = "_QMmFfooEx", test.ptr = "x"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>, !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>)
+  %5:2 = hlfir.declare %arg1 {uniq_name = "_QMmFfooEy", test.ptr = "y"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>, !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>)
+  %6 = hlfir.designate %4#0{"next"}   {fortran_attrs = #fir.var_attrs<pointer>, test.ptr = "xnext1"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> !fir.ref<!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>>
+  %7 = fir.load %6 : !fir.ref<!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>>
+  %8 = fir.box_addr %7 : (!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>) -> !fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>
+  %9 = hlfir.designate %8{"i"} : (!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> !fir.ref<i32>
+  %10 = fir.load %9 : !fir.ref<i32>
+  hlfir.assign %10 to %1#0 : i32, !fir.ref<i32>
+  hlfir.assign %5#0 to %4#0 : !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>, !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>
+  %11 = hlfir.designate %4#0{"next"}   {fortran_attrs = #fir.var_attrs<pointer>, test.ptr = "xnext2"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> !fir.ref<!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>>
+  %12 = fir.load %11 : !fir.ref<!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>>
+  %13 = fir.box_addr %12 : (!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>) -> !fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>
+  %14 = hlfir.designate %13{"i"} : (!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> !fir.ref<i32>
+  %15 = fir.load %14 : !fir.ref<i32>
+  hlfir.assign %15 to %3#0 : i32, !fir.ref<i32>
+  return
+}

>From 9caba5406460e4a14ced2093727b6c7b856d24f0 Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Mon, 6 May 2024 17:12:52 -0700
Subject: [PATCH 5/7] Simplifying initialization of followingData

---
 flang/lib/Optimizer/Analysis/AliasAnalysis.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index ade240bc2364e..fb9f6045586dc 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -251,7 +251,7 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
   bool followBoxData{mlir::isa<fir::BaseBoxType>(v.getType())};
   bool isBoxRef{fir::isa_ref_type(v.getType()) &&
                 mlir::isa<fir::BaseBoxType>(fir::unwrapRefType(v.getType()))};
-  bool followingData = !isBoxRef || followBoxData;
+  bool followingData = !isBoxRef;
   mlir::SymbolRefAttr global;
   Source::Attributes attributes;
   while (defOp && !breakFromLoop) {

>From 60d6c942b797e15b06633d3ef11540ebca99e969 Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Mon, 13 May 2024 13:00:34 -0700
Subject: [PATCH 6/7] Review feedback

---
 .../flang/Optimizer/Analysis/AliasAnalysis.h  | 24 +++++++++++++++----
 .../lib/Optimizer/Analysis/AliasAnalysis.cpp  |  7 +++---
 .../AliasAnalysis/alias-analysis-2.fir        |  2 +-
 .../AliasAnalysis/alias-analysis-3.fir        |  2 +-
 .../AliasAnalysis/alias-analysis-9.fir        | 10 ++++++--
 5 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
index 8a0eb525f1915..b6f18879dd04d 100644
--- a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
+++ b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
@@ -54,9 +54,22 @@ struct AliasAnalysis {
   // the use-def chain, to arrive at the same origin, even though the starting
   // points were known to not alias.
   //
-  // Example:
   // clang-format off
-  //    fir.global @_QMtopEa : !fir.box<!fir.ptr<!fir.array<?xf32>>> 
+  // Example:
+  //  ------------------- test.f90 --------------------
+  //  module top
+  //    real, pointer :: a(:)
+  //  end module
+  //  
+  //  subroutine test()
+  //    use top
+  //    a(1) = 1
+  //  -------------------------------------------------
+  // 
+  //  flang-new -fc1 -emit-fir test.f90 -o test.fir
+  //
+  //  ------------------- test.fir --------------------
+  //  fir.global @_QMtopEa : !fir.box<!fir.ptr<!fir.array<?xf32>>> 
   //  
   //  func.func @_QPtest() {
   //    %c1 = arith.constant 1 : index
@@ -69,10 +82,11 @@ struct AliasAnalysis {
   //    fir.store %cst to %5 : !fir.ref<f32>
   //    return
   //  }
+  //  -------------------------------------------------
   //
   // With high level operations, such as fir.array_coor, it is possible to
-  // reach into the data wrapped by the box (the descriptor) therefore when
-  // asking about the memory source of the %5, we are really asking about the
+  // reach into the data wrapped by the box (the descriptor). Therefore when
+  // asking about the memory source of %5, we are really asking about the
   // source of the data of box %2.
   //
   // When asking about the source of %0 which is the address of the box, we
@@ -88,7 +102,7 @@ struct AliasAnalysis {
   // So in the above example, !fir.ref<f32> and !fir.box<!fir.ptr<!fir.array<?xf32>>> is data, 
   // while !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>> is not data.
 
-  // This generally applies to function arguments. In the example below, %arg0
+  // This also applies to function arguments. In the example below, %arg0
   // is data, %arg1 is not data but a load of %arg1 is.
   // 
   // func.func @_QFPtest2(%arg0: !fir.ref<f32>, %arg1: !fir.ref<!fir.box<!fir.ptr<f32>>> )  {
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index fb9f6045586dc..165ceef60d768 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -173,9 +173,8 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
     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.
+  // POINTER components may alias with POINTER objects,
+  // as well as other POINTER components
   if ((src1->isRecordWithPointerComponent() && src2->isTargetOrPointer()) ||
       (src2->isRecordWithPointerComponent() && src1->isTargetOrPointer()) ||
       (src1->isRecordWithPointerComponent() &&
@@ -290,7 +289,7 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
         .Case<fir::LoadOp>([&](auto op) {
           // If the load is from a leaf source, return the leaf. Do not track
           // through indirections otherwise.
-          // TODO: At support to fir.alloca and fir.allocmem
+          // TODO: Add support to fir.alloca and fir.allocmem
           auto def = getOriginalDef(op.getMemref());
           if (isDummyArgument(def) ||
               def.template getDefiningOp<fir::AddrOfOp>()) {
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
index af3e491e062c3..d03348efd2a68 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
@@ -143,7 +143,7 @@ func.func @_QFPtest2(%arg0: !fir.ref<f32> {fir.bindc_name = "v1", fir.target}, %
 // alias with the wrapped scalar _QFEvar2. We meant box_addr of _QMpointersEp 
 // CHECK-DAG: p#0 <-> box.addr#0: NoAlias
 
-// Now handling more gracefully the difference between !fir.ref<!fir.box<>> and !fir.box<> 
+// Handling gracefully the difference between !fir.ref<!fir.box<>> and !fir.box<> 
 // CHECK-DAG: box.addr#0 <-> func.region0#0: NoAlias
 
 // var2, although it is a target, cannot alias with p
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir
index 3d576cb5c0af9..91829a461dc72 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir
@@ -23,7 +23,7 @@
 // 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
 
-// pointer_dummy's box cannot alias with raw reference to f32 (x), so MayAlias below must be NoAlias:
+// pointer_dummy's box cannot alias with raw reference to f32 (x)
 // CHECK: func.region0#0 <-> func.region0#1: NoAlias
 
 fir.global @_QMmEa : !fir.type<_QMmTt{pointer_component:!fir.box<!fir.ptr<f32>>}> {
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
index 2b92872a2c4b5..8194358d48797 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
@@ -17,14 +17,20 @@
 // end module
 
 // CHECK-LABEL: Testing : "_QMmPfoo"
-// TODO: x and y do not alias
+// TODO: x and y are non pointer, non target argument and therefore do not alias.
 // CHECK-DAG: x#0 <-> y#0: MayAlias
+
+// TODO: y is not a pointer object and therefore does not alias with the x%next component. 
 // CHECK-DAG: y#0 <-> xnext1#0: MayAlias
 // CHECK-DAG: y#0 <-> xnext2#0: MayAlias
 
-// These however do alias
+// Pointer components may alias with pointer objects, 
+// as well as other pointer components
 // CHECK-DAG: x#0 <-> xnext1#0: MayAlias
 // CHECK-DAG: x#0 <-> xnext2#0: MayAlias
+
+// TODO: xnext1#0 <-> xnext2#0 are the same and therefore MustAlias but 
+//       we are currently not comparing operands involved in offset computations
 // CHECK-DAG: xnext1#0 <-> xnext2#0: MayAlias
 
 func.func @_QMmPfoo(%arg0: !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>> {fir.bindc_name = "x"}, %arg1: !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>> {fir.bindc_name = "y"}) {

>From 804a4b968053cc80509406c21748f7b1ee74471c Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Wed, 15 May 2024 14:12:59 -0700
Subject: [PATCH 7/7] Rebased to commit
 83f065d582977aca5c037c27a7290f30850bdd35

---
 flang/include/flang/Optimizer/Analysis/AliasAnalysis.h | 1 +
 flang/lib/Optimizer/Analysis/AliasAnalysis.cpp         | 7 ++++---
 flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir | 6 ++++--
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
index b6f18879dd04d..40fd1705115a0 100644
--- a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
+++ b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
@@ -64,6 +64,7 @@ struct AliasAnalysis {
   //  subroutine test()
   //    use top
   //    a(1) = 1
+  //  end subroutine
   //  -------------------------------------------------
   // 
   //  flang-new -fc1 -emit-fir test.f90 -o test.fir
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index 165ceef60d768..9d0d706a85c5e 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -108,7 +108,7 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
   auto lhsSrc = getSource(lhs);
   auto rhsSrc = getSource(rhs);
   bool approximateSource = lhsSrc.approximateSource || rhsSrc.approximateSource;
-  LLVM_DEBUG(llvm::dbgs() << "\n"; llvm::dbgs() << "AliasAnalysis::alias\n";
+  LLVM_DEBUG(llvm::dbgs() << "\nAliasAnalysis::alias\n";
              llvm::dbgs() << "  lhs: " << lhs << "\n";
              llvm::dbgs() << "  lhsSrc: " << lhsSrc << "\n";
              llvm::dbgs() << "  rhs: " << rhs << "\n";
@@ -173,8 +173,9 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
     return AliasResult::MayAlias;
   }
 
-  // POINTER components may alias with POINTER objects,
-  // as well as other POINTER components
+  // 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() &&
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
index 8194358d48797..df24a6d32aa59 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
@@ -21,11 +21,13 @@
 // CHECK-DAG: x#0 <-> y#0: MayAlias
 
 // TODO: y is not a pointer object and therefore does not alias with the x%next component. 
+//       Also assigning x to y would not modify x.next
 // CHECK-DAG: y#0 <-> xnext1#0: MayAlias
 // CHECK-DAG: y#0 <-> xnext2#0: MayAlias
 
-// Pointer components may alias with pointer objects, 
-// as well as other pointer components
+// We need to catch the fact that assigning y to x will modify xnext. 
+// The only side-effect between the 2 loads of x.next is the assignment to x, 
+// therefore x needs to alias with x.next to prevent the loads from being merged.
 // CHECK-DAG: x#0 <-> xnext1#0: MayAlias
 // CHECK-DAG: x#0 <-> xnext2#0: MayAlias
 



More information about the flang-commits mailing list