[PATCH] D105634: [OpenMP] Use AAHeapToStack/AAHeapToShared analysis in SPMDization

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Test ;)



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2589
+      }
+      // TODO: Do we need to assume there is one, unique free user?
+      if (FreeCalls.size() != 1)
----------------
jhuber6 wrote:
> There should only ever be one free call per variable, it's like a stack. This is here just to make sure we don't accidentally remove something that's behind a PHI or something.
Initially we have one free per alloc and the free uses the alloc, transformations can change that, e.g. introduce phi nodes or sink free into conditionals. We need to be conservative if the structure is not as we expect it.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3432-3435
+      A.getAAFor<AAHeapToStack>(*this, IRPosition::function(*CB.getCaller()),
+                                DepClassTy::REQUIRED);
+      A.getAAFor<AAHeapToShared>(*this, IRPosition::function(*CB.getCaller()),
+                                 DepClassTy::REQUIRED);
----------------
One is sufficient, if the other becomes invalid we don't have to give up. In general, we should not give up on all of kernel info if SPMD-zation can't happen. Also, this is not even needed here ;)


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3479-3481
+        *this, IRPosition::function(*CB.getCaller()), DepClassTy::REQUIRED);
+    auto &HeapToSharedAA = A.getAAFor<AAHeapToShared>(
+        *this, IRPosition::function(*CB.getCaller()), DepClassTy::REQUIRED);
----------------



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3509
+      indicatePessimisticFixpoint();
+    }
+
----------------
`indicateOptimisticFixpoint` is not correct above.
You use "assumed" information, thus it can change over time. If the information is known you could indicate a fixpoint. Instead, just don't add this call site to the `SPMDCompatibilityTracker` if we assume it is removed (by one of the h2s AAs).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105634/new/

https://reviews.llvm.org/D105634