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

[via flang-commits]

================
@@ -97,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()) {
----------------
jeanPerier wrote:

I think this may not be correct with derived types containing POINTERs/ALLOCATABLES.

```
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
```

An optimization trying to merge similar FIR loads would run into trouble if it relied on the alias analysis because it would incorrectly think it can merge the loads of the two `x%next` descriptor loads.

The simplified FIR looks like:

````
 !_QMmTt = !fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>

func.func @_QMmPfoo(%x: !fir.ref<!_QMmTt> {fir.bindc_name = "x"}, %y: !fir.ref<!_QMmTt>) {
  %i1 = fir.alloca i32
  %i2 = fir.alloca i32
  %2 = hlfir.designate %x{"next"}   {fortran_attrs = #fir.var_attrs<pointer>} : (!fir.ref<!_QMmTt>) -> !fir.ref<!fir.box<!fir.ptr<!_QMmTt>>>
  %3 = fir.load %2 : !fir.ref<!fir.box<!fir.ptr<!_QMmTt>>>
  %4 = fir.box_addr %3 : (!fir.box<!fir.ptr<!_QMmTt>>) -> !fir.ptr<!_QMmTt>
  %5 = hlfir.designate %4{"i"}   : (!fir.ptr<!_QMmTt>) -> !fir.ref<i32>
  %6 = fir.load %5 : !fir.ref<i32>
  hlfir.assign %6 to %i1 : i32, !fir.ref<i32>
  hlfir.assign %y to %x : !fir.ref<!_QMmTt>, !fir.ref<!_QMmTt>
  %7 = fir.load %2 : !fir.ref<!fir.box<!fir.ptr<!_QMmTt>>>
  // ... assign to "%i" to i2
  return
}
````

My issue is that I think with the new approach, a pass relying on memory side effects + alias to merge/move fir.load would merge the two fir.load from %2 (%7 and %3) because the only  write effects in between are the two hlfir.assign. The assign to `i1` obviously do not affect %2, but the one to `%x` does, yet since I think `%x` and `%2` are considered to not alias (same source, but %x is "data", while `%2` is not), it would seem legal to merge them.

https://github.com/llvm/llvm-project/pull/91020