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

[Jeroen Ketema via llvm-commits]

jketema 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));
----------------
sbaranga wrote:
> 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.
I'm not sure what you're saying. Sticking with `vld3` Do you mean you both want to expose `@llvm.arm.neon.vld3.v2i32.p0i8` and `@llvm.arm.neon.vld3.v2i32`? Or do you want more exposed?

I'm also not quite sure what you mean by auto-upgrade support?


http://reviews.llvm.org/D12985