[PATCH] Carry facts about nullness and undef across GC relocation

Philip Reames listmail at philipreames.com
Thu Dec 11 15:38:28 PST 2014


Response to Sanjoy's comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:1160
@@ +1159,3 @@
+
+    // Undef is undef, even after relocation.
+    if (isa<UndefValue>(DerivedPtr)) {
----------------
sanjoy wrote:
> While I'm okay with keeping in this clause for now, I'm not comfortable with it.  
> 
> What you're effectively saying here is that given any bit pattern `P`, there a source pattern `Q` such that `P = relocate(Q)`.  But there may be collectors that do not map to specific address patterns.  For example, in x86-64, I think most collectors won't map any address to, say, `((void *) -1)` because that is outside the 48 bit range.
I think this actually is legal.  The bitpattern before relocation can be any bit pattern.  We've got two cases: 
- it's an address on which relocation is well defined (for a given collector), in which case some unknown value (with a possibly restricted value) would be the right result.  Undef is not a valid result in this case.
- it's an address on which relocation is not well defined.  i.e. we could pick it to be relocate(-1)  Given this is not a well defined operation, we are free to pick any bit pattern as output.  Undef is a valid result.

My argument that be that relocating undef can always be chosen to by the second case.  I don't know of a collector with exactly specified relocations for any arbitrary address.  Do you disagree?

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:1167
@@ +1166,3 @@
+    // TODO: provide a hook for this in GCStrategy
+    if (isa<Constant>(DerivedPtr) &&
+        DerivedPtr->getType()->isPointerTy() &&
----------------
sanjoy wrote:
> Isn't this check the same as `isa<ConstantPointerNull>(DerivedPtr)`?
I *think* so, but am not entirely sure.

http://reviews.llvm.org/D6600

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list