Review feedback

Author: Renaud-K

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>>> )  {

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>()) {

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

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>>}> {

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"}) {