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

[Michael Zolotukhin via llvm-commits]

mzolotukhin added inline comments.

================
Comment at: lib/Analysis/ValueTracking.cpp:3302-3303
@@ -3301,3 +3301,4 @@
 
-    if (Offset.isNonNegative())
+    if (BV->getType()->getPointerElementType()->isSized() &&
+        Offset.isNonNegative())
       if (isDereferenceableFromAttribute(BV, Offset, Ty, DL, CtxI, DT, TLI) &&
----------------
apilipenko wrote:
> mzolotukhin wrote:
> > I updated the check to `isSized`.
> > 
> > I'm not well familiar with this code, so I don't have strong opinion on whether these checks are worth pulling in or not. It seems useful though.
> I don't think that you have to sink isNonNegative check because it's checking a contract of stripAndAccumulateInBoundsConstantOffsets function. But it makes sense to sink isSized check to isDereferenceableFromAttribute.
The issue is with `isAligned`, not with `isDereferenceableFromAttribute`.

`isDereferenceableFromAttribute` (ValueTracking.cpp:3118) has an assert
```
assert(Ty->isSized() && "must be sized");
```
But we pass `Ty` there, not `BV->getType()->getPointerElementType()`, that's why the assert doesn't fire.

`isAligned` doesn't have a check or an assert, and we don't pass a type there, so it derives the type from the value we pass (`BV`):
```
    Type *Ty = Base->getType()->getPointerElementType();
    BaseAlign = DL.getABITypeAlignment(Ty);
```
This type `Ty` happens to be opaque, and somewhere in `getABITypeAlignment` we crash.

Will it make sense to add the `isSized` check to `isAligned`?


http://reviews.llvm.org/D15597