[PATCH] D15597: [ValueTracking] Handle opaque types in isDereferenceableAndAlignedPointer.

[Artur Pilipenko via llvm-commits]

apilipenko added a comment.

isAligned indeed requires base pointer to be sized. You either make it a contract of the function and add an assertion in isAligned, or support non-sized base pointers and conservatively return false for them. I'd prefer the latter even if it results in two checks for one of the isAligned versions. Alternatively you will need to verify that any other place where isAligned is called base pointer can't be non-sized.


================
Comment at: lib/Analysis/ValueTracking.cpp:3304
@@ -3303,2 +3303,3 @@
+        Offset.isNonNegative())
       if (isDereferenceableFromAttribute(BV, Offset, Ty, DL, CtxI, DT, TLI) &&
           isAligned(BV, Offset, Align, DL))
----------------
mzolotukhin wrote:
> Gerolf wrote:
> > The call to isDeferenceableFromAttribute does not look correct. Should Ty match BV's type. I think the invocation without the Ty parameter is the one needed here. Accidentally this would fix the assertion bug, too, since the subsequent call to isAligned would not happen.
> If I'm not mistaken, the invocation without the `Ty` parameter goes without the `Offset` parameter as well, which is required in this case. So, we can't use another version of `isDeferenceableFromAttribute ` here.
> 
> That said, the case when BV's type doesn't match V's type is indeed a special case - should we maybe just bailout in this case, or is it expected, and we know how to properly handle it?
Ty other than BV type is a perfectly correct and supported case. You might reference to a field of a struct for example. In this case you will have a struct as a base value (BV) and a field type as Ty.


http://reviews.llvm.org/D15597