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

[Eduard Burtescu via llvm-commits]

eddyb 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());
 
----------------
dblaikie wrote:
> 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?
`getNullValue`, confusingly enough, isn't about `nullptr` but `zeroinitializer`.
And I didn't change the semantics, I just replaced `LI->getPointerOperand()->getPointerElementType()` with `LI->getType()`.

================
Comment at: lib/Transforms/Vectorize/BBVectorize.cpp:2317
@@ -2316,3 @@
-
-    Type *ArgTypeI = IPtr->getType()->getPointerElementType();
-    Type *ArgTypeJ = JPtr->getType()->getPointerElementType();
----------------
dblaikie wrote:
> 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?
Correct - otherwise I would've ended up with unused variables and duplicated work.

================
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;
----------------
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.


http://reviews.llvm.org/D16417