[PATCH] Fix SROA creating invalid bitcasts between address spaces

David Blaikie dblaikie at gmail.com
Wed Oct 16 10:55:22 PDT 2013


On Wed, Oct 16, 2013 at 10:12 AM, Tom Stellard <tom at stellard.net> wrote:

> Hi,
>
> This patch fixes http://www.llvm.org/bugs/show_bug.cgi?id=15907  What
> needs to be done to get it upstream?
>

Just bump the thread for review from time to time (usual rate is weekly, if
it's high priority/you've got enough community credit, you can spend that
to push harder).

- David


>
> -Tom
>
> On Thu, Sep 26, 2013 at 12:46:53PM -0700, Matt Arsenault wrote:
> > Hi chandlerc,
> >
> > http://llvm-reviews.chandlerc.com/D1764
> >
> > Files:
> >   lib/Transforms/Scalar/SROA.cpp
> >   test/Transforms/SROA/address-spaces.ll
> >
> > Index: lib/Transforms/Scalar/SROA.cpp
> > ===================================================================
> > --- lib/Transforms/Scalar/SROA.cpp
> > +++ lib/Transforms/Scalar/SROA.cpp
> > @@ -1416,7 +1416,8 @@
> >
> >    if (!OffsetPtr) {
> >      if (!Int8Ptr) {
> > -      Int8Ptr = IRB.CreateBitCast(Ptr, IRB.getInt8PtrTy(),
> > +      unsigned AS = Ptr->getType()->getPointerAddressSpace();
> > +      Int8Ptr = IRB.CreateBitCast(Ptr, IRB.getInt8PtrTy(AS),
> >                                    "raw_cast");
> >        Int8PtrOffset = Offset;
> >      }
> > @@ -2526,16 +2527,17 @@
> >      IntegerType *SubIntTy
> >        = IntTy ? Type::getIntNTy(IntTy->getContext(), Size*8) : 0;
> >
> > -    Type *OtherPtrTy = NewAI.getType();
> > +    unsigned OtherAS = OtherPtr->getType()->getPointerAddressSpace();
> > +    Type *OtherPtrTy = NewAllocaTy->getPointerTo(OtherAS);
> >      if (VecTy && !IsWholeAlloca) {
> >        if (NumElements == 1)
> >          OtherPtrTy = VecTy->getElementType();
> >        else
> >          OtherPtrTy = VectorType::get(VecTy->getElementType(),
> NumElements);
> >
> > -      OtherPtrTy = OtherPtrTy->getPointerTo();
> > +      OtherPtrTy = OtherPtrTy->getPointerTo(OtherAS);
> >      } else if (IntTy && !IsWholeAlloca) {
> > -      OtherPtrTy = SubIntTy->getPointerTo();
> > +      OtherPtrTy = SubIntTy->getPointerTo(OtherAS);
> >      }
> >
> >      Value *SrcPtr = getAdjustedPtr(IRB, DL, OtherPtr, RelOffset,
> OtherPtrTy);
> > Index: test/Transforms/SROA/address-spaces.ll
> > ===================================================================
> > --- /dev/null
> > +++ test/Transforms/SROA/address-spaces.ll
> > @@ -0,0 +1,51 @@
> > +; RUN: opt < %s -sroa -S | FileCheck %s
> > +target datalayout =
> "e-p:64:64:64-p1:16:16:16-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-n8:16:32:64"
> > +
> > +declare void @llvm.memcpy.p0i8.p0i8.i32(i8*, i8*, i32, i32, i1)
> > +declare void @llvm.memcpy.p1i8.p0i8.i32(i8 addrspace(1)*, i8*, i32,
> i32, i1)
> > +declare void @llvm.memcpy.p0i8.p1i8.i32(i8*, i8 addrspace(1)*, i32,
> i32, i1)
> > +declare void @llvm.memcpy.p1i8.p1i8.i32(i8 addrspace(1)*, i8
> addrspace(1)*, i32, i32, i1)
> > +
> > +; Make sure an illegal bitcast isn't introduced
> > +define void @test_address_space_1_1(<2 x i64> addrspace(1)* %a, i16
> addrspace(1)* %b) {
> > +; CHECK-LABEL: @test_address_space_1_1(
> > +; CHECK: load <2 x i64> addrspace(1)* %a, align 2
> > +; CHECK: store <2 x i64> {{.*}}, <2 x i64> addrspace(1)* {{.*}}, align 2
> > +; CHECK: ret void
> > +  %aa = alloca <2 x i64>, align 16
> > +  %aptr = bitcast <2 x i64> addrspace(1)* %a to i8 addrspace(1)*
> > +  %aaptr = bitcast <2 x i64>* %aa to i8*
> > +  call void @llvm.memcpy.p0i8.p1i8.i32(i8* %aaptr, i8 addrspace(1)*
> %aptr, i32 16, i32 2, i1 false)
> > +  %bptr = bitcast i16 addrspace(1)* %b to i8 addrspace(1)*
> > +  call void @llvm.memcpy.p1i8.p0i8.i32(i8 addrspace(1)* %bptr, i8*
> %aaptr, i32 16, i32 2, i1 false)
> > +  ret void
> > +}
> > +
> > +define void @test_address_space_1_0(<2 x i64> addrspace(1)* %a, i16*
> %b) {
> > +; CHECK-LABEL: @test_address_space_1_0(
> > +; CHECK: load <2 x i64> addrspace(1)* %a, align 2
> > +; CHECK: store <2 x i64> {{.*}}, <2 x i64>* {{.*}}, align 2
> > +; CHECK: ret void
> > +  %aa = alloca <2 x i64>, align 16
> > +  %aptr = bitcast <2 x i64> addrspace(1)* %a to i8 addrspace(1)*
> > +  %aaptr = bitcast <2 x i64>* %aa to i8*
> > +  call void @llvm.memcpy.p0i8.p1i8.i32(i8* %aaptr, i8 addrspace(1)*
> %aptr, i32 16, i32 2, i1 false)
> > +  %bptr = bitcast i16* %b to i8*
> > +  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %bptr, i8* %aaptr, i32 16,
> i32 2, i1 false)
> > +  ret void
> > +}
> > +
> > +define void @test_address_space_0_1(<2 x i64>* %a, i16 addrspace(1)*
> %b) {
> > +; CHECK-LABEL: @test_address_space_0_1(
> > +; CHECK: load <2 x i64>* %a, align 2
> > +; CHECK: store <2 x i64> {{.*}}, <2 x i64> addrspace(1)* {{.*}}, align 2
> > +; CHECK: ret void
> > +  %aa = alloca <2 x i64>, align 16
> > +  %aptr = bitcast <2 x i64>* %a to i8*
> > +  %aaptr = bitcast <2 x i64>* %aa to i8*
> > +  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %aaptr, i8* %aptr, i32 16,
> i32 2, i1 false)
> > +  %bptr = bitcast i16 addrspace(1)* %b to i8 addrspace(1)*
> > +  call void @llvm.memcpy.p1i8.p0i8.i32(i8 addrspace(1)* %bptr, i8*
> %aaptr, i32 16, i32 2, i1 false)
> > +  ret void
> > +}
> > +
>
> > Index: lib/Transforms/Scalar/SROA.cpp
> > ===================================================================
> > --- lib/Transforms/Scalar/SROA.cpp
> > +++ lib/Transforms/Scalar/SROA.cpp
> > @@ -1416,7 +1416,8 @@
> >
> >    if (!OffsetPtr) {
> >      if (!Int8Ptr) {
> > -      Int8Ptr = IRB.CreateBitCast(Ptr, IRB.getInt8PtrTy(),
> > +      unsigned AS = Ptr->getType()->getPointerAddressSpace();
> > +      Int8Ptr = IRB.CreateBitCast(Ptr, IRB.getInt8PtrTy(AS),
> >                                    "raw_cast");
> >        Int8PtrOffset = Offset;
> >      }
> > @@ -2526,16 +2527,17 @@
> >      IntegerType *SubIntTy
> >        = IntTy ? Type::getIntNTy(IntTy->getContext(), Size*8) : 0;
> >
> > -    Type *OtherPtrTy = NewAI.getType();
> > +    unsigned OtherAS = OtherPtr->getType()->getPointerAddressSpace();
> > +    Type *OtherPtrTy = NewAllocaTy->getPointerTo(OtherAS);
> >      if (VecTy && !IsWholeAlloca) {
> >        if (NumElements == 1)
> >          OtherPtrTy = VecTy->getElementType();
> >        else
> >          OtherPtrTy = VectorType::get(VecTy->getElementType(),
> NumElements);
> >
> > -      OtherPtrTy = OtherPtrTy->getPointerTo();
> > +      OtherPtrTy = OtherPtrTy->getPointerTo(OtherAS);
> >      } else if (IntTy && !IsWholeAlloca) {
> > -      OtherPtrTy = SubIntTy->getPointerTo();
> > +      OtherPtrTy = SubIntTy->getPointerTo(OtherAS);
> >      }
> >
> >      Value *SrcPtr = getAdjustedPtr(IRB, DL, OtherPtr, RelOffset,
> OtherPtrTy);
> > Index: test/Transforms/SROA/address-spaces.ll
> > ===================================================================
> > --- /dev/null
> > +++ test/Transforms/SROA/address-spaces.ll
> > @@ -0,0 +1,51 @@
> > +; RUN: opt < %s -sroa -S | FileCheck %s
> > +target datalayout =
> "e-p:64:64:64-p1:16:16:16-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-n8:16:32:64"
> > +
> > +declare void @llvm.memcpy.p0i8.p0i8.i32(i8*, i8*, i32, i32, i1)
> > +declare void @llvm.memcpy.p1i8.p0i8.i32(i8 addrspace(1)*, i8*, i32,
> i32, i1)
> > +declare void @llvm.memcpy.p0i8.p1i8.i32(i8*, i8 addrspace(1)*, i32,
> i32, i1)
> > +declare void @llvm.memcpy.p1i8.p1i8.i32(i8 addrspace(1)*, i8
> addrspace(1)*, i32, i32, i1)
> > +
> > +; Make sure an illegal bitcast isn't introduced
> > +define void @test_address_space_1_1(<2 x i64> addrspace(1)* %a, i16
> addrspace(1)* %b) {
> > +; CHECK-LABEL: @test_address_space_1_1(
> > +; CHECK: load <2 x i64> addrspace(1)* %a, align 2
> > +; CHECK: store <2 x i64> {{.*}}, <2 x i64> addrspace(1)* {{.*}}, align 2
> > +; CHECK: ret void
> > +  %aa = alloca <2 x i64>, align 16
> > +  %aptr = bitcast <2 x i64> addrspace(1)* %a to i8 addrspace(1)*
> > +  %aaptr = bitcast <2 x i64>* %aa to i8*
> > +  call void @llvm.memcpy.p0i8.p1i8.i32(i8* %aaptr, i8 addrspace(1)*
> %aptr, i32 16, i32 2, i1 false)
> > +  %bptr = bitcast i16 addrspace(1)* %b to i8 addrspace(1)*
> > +  call void @llvm.memcpy.p1i8.p0i8.i32(i8 addrspace(1)* %bptr, i8*
> %aaptr, i32 16, i32 2, i1 false)
> > +  ret void
> > +}
> > +
> > +define void @test_address_space_1_0(<2 x i64> addrspace(1)* %a, i16*
> %b) {
> > +; CHECK-LABEL: @test_address_space_1_0(
> > +; CHECK: load <2 x i64> addrspace(1)* %a, align 2
> > +; CHECK: store <2 x i64> {{.*}}, <2 x i64>* {{.*}}, align 2
> > +; CHECK: ret void
> > +  %aa = alloca <2 x i64>, align 16
> > +  %aptr = bitcast <2 x i64> addrspace(1)* %a to i8 addrspace(1)*
> > +  %aaptr = bitcast <2 x i64>* %aa to i8*
> > +  call void @llvm.memcpy.p0i8.p1i8.i32(i8* %aaptr, i8 addrspace(1)*
> %aptr, i32 16, i32 2, i1 false)
> > +  %bptr = bitcast i16* %b to i8*
> > +  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %bptr, i8* %aaptr, i32 16,
> i32 2, i1 false)
> > +  ret void
> > +}
> > +
> > +define void @test_address_space_0_1(<2 x i64>* %a, i16 addrspace(1)*
> %b) {
> > +; CHECK-LABEL: @test_address_space_0_1(
> > +; CHECK: load <2 x i64>* %a, align 2
> > +; CHECK: store <2 x i64> {{.*}}, <2 x i64> addrspace(1)* {{.*}}, align 2
> > +; CHECK: ret void
> > +  %aa = alloca <2 x i64>, align 16
> > +  %aptr = bitcast <2 x i64>* %a to i8*
> > +  %aaptr = bitcast <2 x i64>* %aa to i8*
> > +  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %aaptr, i8* %aptr, i32 16,
> i32 2, i1 false)
> > +  %bptr = bitcast i16 addrspace(1)* %b to i8 addrspace(1)*
> > +  call void @llvm.memcpy.p1i8.p0i8.i32(i8 addrspace(1)* %bptr, i8*
> %aaptr, i32 16, i32 2, i1 false)
> > +  ret void
> > +}
> > +
>
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131016/57e2adbd/attachment.html>


More information about the llvm-commits mailing list