[PATCH] D135376: [Transforms/ObjCARC] Fix non-deterministic output of `ObjCARCOptPass`

[Argyrios Kyrtzidis via Phabricator via llvm-commits]

akyrtzi marked 2 inline comments as done.
akyrtzi added inline comments.


================
Comment at: llvm/lib/Transforms/ObjCARC/ProvenanceAnalysis.cpp:81
 
-  // Check each unique source of the PHI node against B.
-  SmallPtrSet<const Value *, 4> UniqueSrc;
-  for (Value *PV1 : A->incoming_values()) {
-    if (UniqueSrc.insert(PV1).second && related(PV1, B))
+    if (comparePHISources(PNB, A))
       return true;
----------------
ahatanak wrote:
> akyrtzi wrote:
> > ahatanak wrote:
> > > This function returns true if either `comparePHISources(PNB, A)` or `comparePHISources(A, B)` returns true. Should it return false if either `comparePHISources(PNB, A)` or `comparePHISources(A, B)` returns false instead? I think `related` returns true when the two pointers are related but also when it doesn't know the answer (i.e., it's being conservative). On the other hand, it returns false only when it's certain that the two pointers aren't related.
> > To make sure I understand, the semantics we want here is that two `PHINode`s are only considered related if they //both// have a source value that is related to the other, and if only one `PHINode` has a source value related to the other, but not the other `PHINode`, then they should not be considered related. Is this correct?
> Yes, that was what I was thinking.
Thank you for the feedback! 🙇🏻‍♂️ See updated patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135376