[PATCH] D12985: [ARM] Take into account address spaces in interleaved access vectorization

[silviu.baranga@arm.com via llvm-commits]

sbaranga added inline comments.

================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:11708
@@ +11707,3 @@
+  Type *Int8Ptr = Builder.getInt8PtrTy();
+  Ops.push_back(Builder.CreateAddrSpaceCast(
+      Builder.CreateBitCast(LI->getPointerOperand(), Int8PtrAddr), Int8Ptr));
----------------
jketema wrote:
> sbaranga wrote:
> > jketema wrote:
> > > sbaranga wrote:
> > > > This is interesting.. We ideally wouldn't want to introduce address space casts when not necessary (I can imagine some users using the address space qualifier to help with alias analysis).
> > > > 
> > > > We should be able to get a variant of the vldn intrinsic that operates on the correct address space.
> > > Maybe. I would need some pointers on how to change the code in that case, because this would require changes to `IntrinsicsARM.td`, which I'm not very familiar with.
> > > 
> > > I'm also wondering: is alias analysis information still being used during instruction lowering or later?
> > I've looked into this and changing IntrinisicsARM.td should be fairly simple, all that's needed is to replace in definitions like  int_arm_neon_vld2 the llvm_ptr_ty argument with a llvm_anyptr_ty.
> > 
> > However it turns out that this changes the intrinisic name mangling (which might be a problem with backward compatibility) and also requires clang changes. This is of course a bit complicated..
> Thanks for the pointer regarding llvm_anyptr_ty.
> 
> Maybe map, say, `__builtin_neon_vld3_v` to `@llvm.arm.neon.vld3.v2i32.p0i8` instead of `@llvm.arm.neon.vld3.v2i32` and keep that as the only exposed version? Or are there objections to that solution too?
Yes, we would have to do that and also add auto-upgrade support for the old forms of the intrinsics. Perhaps an email should be sent to llvm-dev as well.


http://reviews.llvm.org/D12985