[PATCH] D16417: [opaque pointer types] [NFC] {Load, Store}Inst: get loaded/stored type from the instruction.

[Manuel Jacob via llvm-commits]

mjacob added inline comments.

================
Comment at: lib/Target/PowerPC/PPCLoopPreIncPrep.cpp:209
@@ -205,4 +208,3 @@
       // There are no update forms for Altivec vector load/stores.
-      if (ST && ST->hasAltivec() &&
-          PtrValue->getType()->getPointerElementType()->isVectorTy())
+      if (ST && ST->hasAltivec() && ValTy && ValTy->isVectorTy())
         continue;
----------------
I think you changed semantics here. What if the instruction is a prefetch intrinsic with a pointer to a vector as the first operand?

================
Comment at: lib/Transforms/IPO/GlobalOpt.cpp:2437
@@ -2436,2 +2436,3 @@
       }
-      if (!isSimpleEnoughPointerToCommit(Ptr)) {
+      Type *ValTy = SI->getValueOperand()->getType();
+      if (!isSimpleEnoughPointerToCommit(Ptr, ValTy)) {
----------------
Do we need this variable?

================
Comment at: lib/Transforms/Scalar/MemCpyOptimizer.cpp:236
@@ -230,2 +235,3 @@
   void addMemSet(int64_t OffsetFromFirst, MemSetInst *MSI) {
+    Type *DestTy = Type::getInt8Ty(MSI->getContext());
     int64_t Size = cast<ConstantInt>(MSI->getLength())->getZExtValue();
----------------
I think it's slightly more robost to get the type with `MSI->getValue()->getType()`.

================
Comment at: lib/Transforms/Scalar/Scalarizer.cpp:268-270
@@ -270,3 +267,5 @@
 // vector or a pointer to a vector.
-Scatterer Scalarizer::scatter(Instruction *Point, Value *V) {
+Scatterer Scalarizer::scatter(Instruction *Point, Value *V, Type *VecTy) {
+  if (!VecTy)
+    VecTy = V->getType();
   if (Argument *VArg = dyn_cast<Argument>(V)) {
----------------
Can you document (and possibly add an assertion for) the requirements on VecTy?

If I understand correctly, there are two possibilities:
1) `V` is a vector and `VecTy` is nullptr.
2) `V` is a pointer to a vector and `VecTy` is the pointee type.

At least we should add an assertion that `V` is a vector if `VecTy` is nullptr.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1991
@@ -1990,3 +1990,3 @@
   // Make sure that the pointer does not point to structs.
-  if (Ptr->getType()->getPointerElementType()->isAggregateType())
+  if (DataType && DataType->isAggregateType())
     return 0;
----------------
eddyb wrote:
> eddyb wrote:
> > dblaikie wrote:
> > > Do we need to support a null type parameter here?
> > This might be an artifact of rebasing, i.e. it was needed but now it's not.
> > I'll try testing without the `nullptr` default and this check.
> Ah, that call site doesn't show up in the commit because it hasn't changed.
> See the `collectLoopUniforms` method.
What if `isConsecutivePtr` is called from `collectLoopUniforms` with a pointer to a struct?


http://reviews.llvm.org/D16417