[flang-commits] [flang] [flang] Respect record type changes in FIR alias analysis access paths (PR #231655)

[Slava Zakharin via flang-commits]

================
@@ -591,40 +669,43 @@ static bool noAliasBasedOnType(mlir::Value lhs, mlir::Value rhs) {
 /// source's POINTER attribute reflects the component traversed during the
 /// walk, not the root variable, so we check only TARGET on the source.
 ///
-/// One exception: if BOTH sides end with a PointerDeref, the two pointers
-/// could independently be associated with the same third-party TARGET
-/// variable, so we conservatively return false.
+/// One exception: if BOTH sides continue through a PointerDeref, the two
+/// pointers could independently be associated with the same third-party
+/// TARGET variable, so such steps do not prove disjointness.
 static bool pathsDivergeAtComponent(const fir::AliasAnalysis::Source &lhsSrc,
                                     const fir::AliasAnalysis::Source &rhsSrc) {
   using PathStep = fir::AliasAnalysis::Source::PathStep;
+  if (lhsSrc.accessPath.storageDecl != rhsSrc.accessPath.storageDecl)
+    return false;
   auto &lhsSteps = lhsSrc.accessPath.steps;
   auto &rhsSteps = rhsSrc.accessPath.steps;
-  size_t minLen = std::min(lhsSteps.size(), rhsSteps.size());
-  for (size_t i = 0; i < minLen; ++i) {
-    if (lhsSteps[i].kind == PathStep::Kind::Component &&
-        rhsSteps[i].kind == PathStep::Kind::Component &&
-        lhsSteps[i].component != rhsSteps[i].component) {
-      auto hasPtrDerefAfter = [](llvm::ArrayRef<PathStep> steps, size_t from) {
-        for (size_t j = from; j < steps.size(); ++j)
-          if (steps[j].kind == PathStep::Kind::PointerDeref)
-            return true;
-        return false;
-      };
-      bool lhsHasPtrDeref = hasPtrDerefAfter(lhsSteps, i + 1);
-      bool rhsHasPtrDeref = hasPtrDerefAfter(rhsSteps, i + 1);
+  auto hasPtrDeref = [](llvm::ArrayRef<PathStep> steps) {
+    return llvm::any_of(steps, [](const PathStep &s) {
+      return s.kind == PathStep::Kind::PointerDeref;
+    });
+  };
+  for (size_t i = 0; i < lhsSteps.size(); ++i) {
+    if (lhsSteps[i].kind != PathStep::Kind::Component)
+      continue;
+    for (size_t j = 0; j < rhsSteps.size(); ++j) {
+      if (rhsSteps[j].kind != PathStep::Kind::Component ||
+          !componentsAreDisjoint(lhsSteps[i], rhsSteps[j]))
+        continue;
+      bool lhsHasPtrDeref =
+          hasPtrDeref(llvm::ArrayRef<PathStep>(lhsSteps).drop_front(i + 1));
+      bool rhsHasPtrDeref =
+          hasPtrDeref(llvm::ArrayRef<PathStep>(rhsSteps).drop_front(j + 1));
       if (lhsHasPtrDeref && rhsHasPtrDeref)
-        return false;
+        continue;
       if (lhsHasPtrDeref || rhsHasPtrDeref) {
-        for (size_t j = 0; j < i; ++j)
-          if (lhsSteps[j].kind == PathStep::Kind::PointerDeref)
-            return false;
+        if (hasPtrDeref(llvm::ArrayRef<PathStep>(lhsSteps).take_front(i)) ||
+            hasPtrDeref(llvm::ArrayRef<PathStep>(rhsSteps).take_front(j)))
+          continue;
         if (lhsSrc.isTarget() || rhsSrc.isTarget())
----------------
vzakhari wrote:

Thanks! This is a real bug, and the TARGET attribute is not needed to hit it. Every component step of one path was compared with every component step of the other path, so the component `a` of `x` was compared with the component `v` of `x%p`, which belongs to another object (`x%a` in your program). Since `child` extends `base`, and neither component contains an object of the other type (`a` only holds a descriptor), the two steps were found disjoint, and the analysis returned NoAlias for `x%a%v` and `x%p%v`. The same happens for `x%a%v(2:4) = x%a%v(1:3)` without any POINTER or TARGET, and for a recursive allocatable component: `x%next%v(2:4) = x%next%v(1:3)`, where `next` is a `type(t), allocatable` component of `t`. All three programs printed wrong results at -O2.

Two component steps are now compared only if they select components of the same object: if neither of them follows a PointerDeref or AllocDeref, or if the steps up to and including the last dereference before each of them are the same. I added the three cases to alias-analysis-access-path-record-type.fir.

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