[PATCH] D20764: Reduce dependence on pointee types when deducing dereferenceability

[Sanjoy Das via llvm-commits]

sanjoy added inline comments.

================
Comment at: lib/Analysis/Loads.cpp:145-146
@@ -175,11 +144,4 @@
 
-  if (Ty->isSized()) {
-    APInt Offset(DL.getTypeStoreSizeInBits(VTy), 0);
-    const Value *BV = V->stripAndAccumulateInBoundsConstantOffsets(DL, Offset);
-
-    if (Offset.isNonNegative())
-      if (isDereferenceableFromAttribute(BV, Offset, Ty, DL, CtxI, DT, TLI) &&
-          isAligned(BV, Offset, Align, DL))
-        return true;
-  }
+  if (!Ty->isSized())
+    return false;
 
----------------
apilipenko wrote:
> You change the behavior for opaque types here. Is it intentional? Previous version tried to give an answer for the cases where it could guarantee dereferenceability, e.g. load from an opaque typed global. I thought that we even have a test case for that.
> 
> If we don't want to support this case anymore, the code can be simplified further. Value::isPointerDereferenceable can be removed, instead we can rely on getPointerDereferenceable to return dereferenceable bytes for the cases now handled by isPointerDereferenceable.
> 
> If we do want to support both sized and opaque types it might make sense to separate handling into two functions, one for sized one for non-sized.
This wasn't intentional, but I also don't remember any upstream llvm-lit tests failing (which is why I thought the change was harmless).  I'll take a closer look tomorrow.


http://reviews.llvm.org/D20764