[PATCH] D41006: [SafepointIRVerifier] Allow non-dereferencing uses of unrelocated or poisoned PHI nodes

[Daniil Suchkov via Phabricator via llvm-commits]

DaniilSuchkov added inline comments.


================
Comment at: lib/IR/SafepointIRVerifier.cpp:262
+/// R - relocated and P - poisoned. Merge with constant pointer doesn't change
+/// anything. Merge of pointers by itself is always safe, deriving one pointer
+/// from another is always safe too. Derived pointer always has the same state
----------------
anna wrote:
> As mentioned, I dont think you need to explicitly state out the distinction here. Just maybe a single line at the beginning when explaining `MultiSourceDerivedPtr` (I prefer that instead of `Poisoned` naming), because we can have multiple sources and still not be poisoned.
We don't need `MultiSourceDerivedPtr` because we don't care at all from how many sources a value was derived, but we do care if all sources were (un)relocated or not. Value which was derived from multiple sources can be in any of three possible states (relocated, unrelocated, poisoned). 


================
Comment at: lib/IR/SafepointIRVerifier.cpp:279
+///         \     |
+///      p2 = phi [p] [p1]
+///      p3 = phi [p] [p]
----------------
anna wrote:
> mkazantsev wrote:
> > anna wrote:
> > > This is incorrect IR. We cannot have multiple *different* incoming phi values from exactly one predecessor.
> > > With correct IR, I don't think we will have such false positives. If we do have, could you please add a test with FIXME?
> > I don't get why it is incorrect. For example,
> > 
> >   void *p = &a[10];
> >   void *p1 = &p[20];
> >   void *temp = p;
> >   if (cond) {
> >     temp = p1;
> >     some_call();
> >   }
> >   void *p2 = temp;
> > 
> > Won't we have exactly this IR here?
> yup, you're right. What we have is different incoming values from different incoming blocks, something like: `p2 = phi [p, def BB of p], [p1, safepoint block]`
> What's incorrect is, different incoming values from the same block. For example, p and p1 from their same def block: `p2 = phi [p, def BB of p] [p1, def BB of p1]`. 
To avoid confusion I'll make this comment a bit more clear.


https://reviews.llvm.org/D41006