[PATCH] D137341: [VectorCombine] widen a load with subvector insert

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:321-322
+  IRBuilder<> Builder(Load);
+  Value *CastedPtr =
+      Builder.CreatePointerBitCastOrAddrSpaceCast(SrcPtr, Ty->getPointerTo(AS));
+  Value *VecLd = Builder.CreateAlignedLoad(Ty, CastedPtr, Alignment);
----------------
spatel wrote:
> arsenm wrote:
> > Shouldn't need to create an addrspacecast here
> I copied this line from the existing fold for a load+insertelt, and that was last changed with D121787. 
> 
> Is that not relevant with this transform and/or the change to opaque pointers? I'm not familiar with all of the addrspace corner-cases, so I'm not sure what to do here. 
> 
> Add tests derived from D121787?
> 
> 
> ```
> define <4 x i32> @load_from_other_as(ptr addrspace(5) align 16 dereferenceable(16) %p) {
>   %asc = addrspacecast ptr addrspace(5) %p to ptr
>   %l = load <2 x i32>, ptr %asc, align 4
>   %s = shufflevector <2 x i32> %l, <2 x i32> poison, <4 x i32> <i32 0, i32 1, i32 undef, i32 undef>
>   ret <4 x i32> %s
> }
> 
> ```
> 
> 
> 
Opaque pointers are unrelated to the address space; there's no change there.

I don't see how it's relevant for this transform. You're widening a load, which should always be into a load with the same address space that the original load used. The only cast you should need here is the element bitcast for typed pointers


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137341/new/

https://reviews.llvm.org/D137341