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

[David Blaikie via llvm-commits]

dblaikie added inline comments.

================
Comment at: lib/CodeGen/AtomicExpandPass.cpp:271
@@ -270,4 +270,3 @@
   Value *Addr = LI->getPointerOperand();
-  Type *Ty = cast<PointerType>(Addr->getType())->getElementType();
-  Constant *DummyVal = Constant::getNullValue(Ty);
+  Constant *DummyVal = Constant::getNullValue(LI->getType());
 
----------------
I must be missing something as this doesn't look quite right - why was the original code treating the load as though it were a load of a pointer? (load's IR type should be a value type, not a pointer type - unless it is loading a pointer (ie: load could be passed a pointer to a pointer)) & why is the new code doing something different?

================
Comment at: lib/Transforms/Vectorize/BBVectorize.cpp:993
@@ -992,3 +997,1 @@
 
-        Type *aTypeI = isa<StoreInst>(I) ?
-          cast<StoreInst>(I)->getValueOperand()->getType() : I->getType();
----------------
I take it this code is still correct - but you chose to plumb the two out params into getPairPtrInfo just so this code didn't have to do all the casting and conditionals, etc? (since you did have to add JTy/ITy to the implementation of getPairPtrInfo anyway, yes?)

================
Comment at: lib/Transforms/Vectorize/BBVectorize.cpp:2317
@@ -2316,3 @@
-
-    Type *ArgTypeI = IPtr->getType()->getPointerElementType();
-    Type *ArgTypeJ = JPtr->getType()->getPointerElementType();
----------------
Same here, I take it? Or was this the part where it was necessary for getPairPtrInfo to produce these two values - and since you were producing them here you might as well use them in the other call site?

================
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;
----------------
Do we need to support a null type parameter here?


http://reviews.llvm.org/D16417