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

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: lib/IR/SafepointIRVerifier.cpp:250
+/// in a very limited number of situations. Currently the only way to use it is
+/// comparison against constant exclusively derived from null. All limitations
+/// arise due to its undefined state: this pointers should be treated as
----------------
DaniilSuchkov wrote:
> mkazantsev wrote:
> > DaniilSuchkov wrote:
> > > mkazantsev wrote:
> > > > You can always represent any constant pointer as `gep null, %some_int_constant`, so I think that this "exclusively derived from null" stuff is redundant.
> > > It refers to comment at line 639.
> > I'm not asking to do something about it in this patch since it was there before, but it is fishy. If I can imagine a VM in which 0xFF is some special magical pointer that cannot be simply compared against normal pointers, then I can also imagine a VM where `gep null, 0xFF` is also some special magical pointer with same properties. Actually, I can define all such special numbers as derivatives from null.
> > 
> > From that perspective, how hard-coded constants are different from hard-coded offsets from null?
> I don't know either, but the idea is to keep this patch consistent with previous code. So I have to maintain the logic around "magic pointer constant". This patch is not about that issue, let's discuss it later.
I'll try to clarify this. The GC relocates the *base* pointer and this is why we record the base pointer for every 'derived pointer'. After the GC relocates the base pointer at runtime, we can rematerialize the derived pointer because we have stored this information in the IR.

So, effectively it comes down to always identifying the "base" of a derived pointer. This is where the `getBaseType` comes in.
When we generate "magic const pointers" in IR (for example, using inttoptr `magic const`), the base here is that magic const. 
The same idiom is `GEP(null, magic const)`, but here the base pointer is null. Relocating a null is still a null.

So, this is why we have something like this:
```
%ptr = unrelocated non constant pointer
 compare (%ptr, inttoptr(magic_const)) <-- can't be reordered before a safepoint
but:
compare(%ptr, GEP(null, magic_const)) <-- can be reordered before a safepoint
```
 Also, just as an aside, this is also why inttoptr of addrspace(1) is incorrect in the IRVerifier, but GEP(null, offset) in addrspace(1) is fine. 


https://reviews.llvm.org/D41006