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

[David Blaikie via llvm-commits]

On Thu, Jan 21, 2016 at 3:26 PM, Eduard Burtescu via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> 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()`.
>

Right - I see I think I misread it, not seeing the difference between
"Addr" in the original formulation, and "LI" in the new formulation - I
thought those were the same "getType" calls, and they aren't. Thanks for
explaining.


>
> ================
> 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
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160121/bfe81a43/attachment.html>