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

Sanjoy Das sanjoy at playingwithpointers.com
Thu Dec 11 12:37:13 PST 2014


LGTM with comments addressed.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:1160
@@ +1159,3 @@
+
+    // Undef is undef, even after relocation.
+    if (isa<UndefValue>(DerivedPtr)) {
----------------
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.

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:1165
@@ +1164,3 @@
+
+    // The relocation of null will be null for most any collector.
+    // TODO: provide a hook for this in GCStrategy
----------------
Nit: should this be "almost any collector"?

================
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() &&
----------------
Isn't this check the same as `isa<ConstantPointerNull>(DerivedPtr)`?

http://reviews.llvm.org/D6600

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






More information about the llvm-commits mailing list