[PATCH] D16272: [opaque pointer types] Alloca: use getAllocatedType() instead of getType()->getElementType().

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 17 13:32:35 PST 2016


dblaikie accepted this revision.
dblaikie added a reviewer: dblaikie.
dblaikie added a comment.
This revision is now accepted and ready to land.

Thanks for all the work/patches posted here. Will be getting to the reviews as quick as I can.

This one looks good, please commit!


================
Comment at: lib/Transforms/Scalar/ScalarReplAggregates.cpp:2237
@@ -2237,4 +2236,3 @@
       uint64_t EltOffset;
-      PointerType *OtherPtrTy = cast<PointerType>(OtherPtr->getType());
-      Type *OtherTy = OtherPtrTy->getElementType();
+      Type *OtherTy = AI->getAllocatedType();
       if (StructType *ST = dyn_cast<StructType>(OtherTy)) {
----------------
eddyb wrote:
> mjacob wrote:
> > Are you sure this is right? I don't think we can assume that `AI` and `OtherPtr` have the same type.
> `OtherPtr` is ensured above to have the type `PointerType::get(AI->getAllocatedType(), AddrSpace)`.
Could throw in an assert to that effect if you like

================
Comment at: lib/Transforms/Scalar/ScalarReplAggregates.cpp:2254
@@ -2255,3 +2253,3 @@
     Value *EltPtr = NewElts[i];
-    Type *EltTy = cast<PointerType>(EltPtr->getType())->getElementType();
+    Type *EltTy = NewElts[i]->getAllocatedType();
 
----------------
eddyb wrote:
> mjacob wrote:
> > Why don't you reuse `EltPtr`?
> Wrong type, `NewElts` is an array of `AllocaInst*`, and I believe `EltPtr` is assigned something that is not an `AllocaInst*` (or used to, looking again at it doesn't seem to be the case atm).
Feel free to correct the type (since, as you say, it's no longer reused for other things) & reuse it if you like


http://reviews.llvm.org/D16272





More information about the llvm-commits mailing list