[PATCH] Teach CodeGenPrepare about address spaces

[Tom Stellard]

On Thu, Sep 05, 2013 at 03:00:33PM -0700, Matt Arsenault wrote:
> http://llvm-reviews.chandlerc.com/D1611
> 
> Files:
>   lib/Transforms/Scalar/CodeGenPrepare.cpp
>   test/CodeGen/R600/address-space.ll
> 
> Index: lib/Transforms/Scalar/CodeGenPrepare.cpp
> ===================================================================
> --- lib/Transforms/Scalar/CodeGenPrepare.cpp
> +++ lib/Transforms/Scalar/CodeGenPrepare.cpp
> @@ -1031,11 +1031,12 @@
>    switch (Opcode) {
>    case Instruction::PtrToInt:
>      // PtrToInt is always a noop, as we know that the int type is pointer sized.
> +    // XXX - Why do we know this?
>      return MatchAddr(AddrInst->getOperand(0), Depth);
>    case Instruction::IntToPtr:
>      // This inttoptr is a no-op if the integer type is pointer sized.
>      if (TLI.getValueType(AddrInst->getOperand(0)->getType()) ==
> -        TLI.getPointerTy())
> +        TLI.getPointerTy(AddrInst->getType()->getPointerAddressSpace()))
>        return MatchAddr(AddrInst->getOperand(0), Depth);
>      return false;
>    case Instruction::BitCast:
> @@ -1573,9 +1574,7 @@
>    } else {
>      DEBUG(dbgs() << "CGP: SINKING nonlocal addrmode: " << AddrMode << " for "
>                   << *MemoryInst);
> -    Type *IntPtrTy =
> -          TLI->getDataLayout()->getIntPtrType(AccessTy->getContext());
> -
> +    Type *IntPtrTy = TLI->getDataLayout()->getIntPtrType(Addr->getType());
>      Value *Result = 0;
>  
>      // Start with the base register. Do this first so that subsequent address
> Index: test/CodeGen/R600/address-space.ll
> ===================================================================
> --- /dev/null
> +++ test/CodeGen/R600/address-space.ll
> @@ -0,0 +1,30 @@
> +; RUN: llc -march=r600 -mcpu=SI < %s | FileCheck %s
> +
> +; Test that codegenprepare understands address space sizes
> +
> +%struct.foo = type { [3 x float], [3 x float] }
> +
> +define void @do_as_ptr_calcs(%struct.foo addrspace(3)* nocapture %ptr) nounwind {
> +; CHECK-LABEL: @do_as_ptr_calcs:
> +; CHECK: V_ADD_I32_e64 VGPR0,
> +; CHECK: V_ADD_I32_e64 VGPR1,
> +; CHECK: DS_READ_B32 VGPR1,

The convention in R600 is to put the CHECK lines above the function
definition.  I'm not sure this matters, but it's nice to stay
consistent.  Also, we tend to use regular expressions for the register
number since they tend to change frequently e.g:

CHECK: V_ADD_I32_e64 VGPR{{[0-9]}},

Otherwise, LGTM.

-Tom

> +entry:
> +  %x = getelementptr inbounds %struct.foo addrspace(3)* %ptr, i32 0, i32 1, i32 0
> +  %y = getelementptr inbounds %struct.foo addrspace(3)* %ptr, i32 0, i32 1, i32 2
> +  br label %bb32
> +
> +bb32:
> +  %a = load float addrspace(3)* %x, align 4
> +  %b = load float addrspace(3)* %y, align 4
> +  %cmp = fcmp one float %a, %b
> +  br i1 %cmp, label %bb34, label %bb33
> +
> +bb33:
> +  unreachable
> +
> +bb34:
> +  unreachable
> +}
> +
> +

> Index: lib/Transforms/Scalar/CodeGenPrepare.cpp
> ===================================================================
> --- lib/Transforms/Scalar/CodeGenPrepare.cpp
> +++ lib/Transforms/Scalar/CodeGenPrepare.cpp
> @@ -1031,11 +1031,12 @@
>    switch (Opcode) {
>    case Instruction::PtrToInt:
>      // PtrToInt is always a noop, as we know that the int type is pointer sized.
> +    // XXX - Why do we know this?
>      return MatchAddr(AddrInst->getOperand(0), Depth);
>    case Instruction::IntToPtr:
>      // This inttoptr is a no-op if the integer type is pointer sized.
>      if (TLI.getValueType(AddrInst->getOperand(0)->getType()) ==
> -        TLI.getPointerTy())
> +        TLI.getPointerTy(AddrInst->getType()->getPointerAddressSpace()))
>        return MatchAddr(AddrInst->getOperand(0), Depth);
>      return false;
>    case Instruction::BitCast:
> @@ -1573,9 +1574,7 @@
>    } else {
>      DEBUG(dbgs() << "CGP: SINKING nonlocal addrmode: " << AddrMode << " for "
>                   << *MemoryInst);
> -    Type *IntPtrTy =
> -          TLI->getDataLayout()->getIntPtrType(AccessTy->getContext());
> -
> +    Type *IntPtrTy = TLI->getDataLayout()->getIntPtrType(Addr->getType());
>      Value *Result = 0;
>  
>      // Start with the base register. Do this first so that subsequent address
> Index: test/CodeGen/R600/address-space.ll
> ===================================================================
> --- /dev/null
> +++ test/CodeGen/R600/address-space.ll
> @@ -0,0 +1,30 @@
> +; RUN: llc -march=r600 -mcpu=SI < %s | FileCheck %s
> +
> +; Test that codegenprepare understands address space sizes
> +
> +%struct.foo = type { [3 x float], [3 x float] }
> +
> +define void @do_as_ptr_calcs(%struct.foo addrspace(3)* nocapture %ptr) nounwind {
> +; CHECK-LABEL: @do_as_ptr_calcs:
> +; CHECK: V_ADD_I32_e64 VGPR0,
> +; CHECK: V_ADD_I32_e64 VGPR1,
> +; CHECK: DS_READ_B32 VGPR1,
> +entry:
> +  %x = getelementptr inbounds %struct.foo addrspace(3)* %ptr, i32 0, i32 1, i32 0
> +  %y = getelementptr inbounds %struct.foo addrspace(3)* %ptr, i32 0, i32 1, i32 2
> +  br label %bb32
> +
> +bb32:
> +  %a = load float addrspace(3)* %x, align 4
> +  %b = load float addrspace(3)* %y, align 4
> +  %cmp = fcmp one float %a, %b
> +  br i1 %cmp, label %bb34, label %bb33
> +
> +bb33:
> +  unreachable
> +
> +bb34:
> +  unreachable
> +}
> +
> +

> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits