[PATCH 3/5] R600: Convert non global value const loads to global loads

[Matt Arsenault]

> On May 25, 2015, at 5:08 PM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
> 
> This seems more complicated, but const laods are getting their own instructions
> in the next commit

Typo laods in message

> 
> Signed-off-by: Jan Vesely <jan.vesely at rutgers.edu <mailto:jan.vesely at rutgers.edu>>
> ---
> lib/Target/R600/AMDGPUISelDAGToDAG.cpp |  4 ++--
> lib/Target/R600/R600ISelLowering.cpp   | 35 ++++++++++++++++++++++++----------
> 2 files changed, 27 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/Target/R600/AMDGPUISelDAGToDAG.cpp b/lib/Target/R600/AMDGPUISelDAGToDAG.cpp
> index df4461e..bba753f 100644
> --- a/lib/Target/R600/AMDGPUISelDAGToDAG.cpp
> +++ b/lib/Target/R600/AMDGPUISelDAGToDAG.cpp
> @@ -649,9 +649,9 @@ bool AMDGPUDAGToDAGISel::isConstantLoad(const LoadSDNode *N, int CbId) const {
> }
> 
> bool AMDGPUDAGToDAGISel::isGlobalLoad(const LoadSDNode *N) const {
> +  //TODO: Why do we need this?
>   if (N->getAddressSpace() == AMDGPUAS::CONSTANT_ADDRESS)
> -    if (Subtarget->getGeneration() < AMDGPUSubtarget::SOUTHERN_ISLANDS ||
> -        N->getMemoryVT().bitsLT(MVT::i32))
> +    if (N->getMemoryVT().bitsLT(MVT::i32))
>       return true;

We shouldn’t. I’ve been wanting to remove the is*Load functions and replace them with more normal PatFrags. The memory type checking here is also very odd

> 
>   return checkType(N->getMemOperand()->getValue(), AMDGPUAS::GLOBAL_ADDRESS);
> diff --git a/lib/Target/R600/R600ISelLowering.cpp b/lib/Target/R600/R600ISelLowering.cpp
> index 8357b6d..33909bf 100644
> --- a/lib/Target/R600/R600ISelLowering.cpp
> +++ b/lib/Target/R600/R600ISelLowering.cpp
> @@ -1469,18 +1469,33 @@ SDValue R600TargetLowering::LowerLOAD(SDValue Op, SelectionDAG &DAG) const
>   }
> 
>   // Lower loads constant address space global variable loads
> -  if (LoadNode->getAddressSpace() == AMDGPUAS::CONSTANT_ADDRESS &&
> -      isa<GlobalVariable>(GetUnderlyingObject(
> +  if ((LoadNode->getAddressSpace() == AMDGPUAS::CONSTANT_ADDRESS) &&
> +      (LoadNode->getExtensionType() != ISD::SEXTLOAD)) {
> +     if (isa<GlobalVariable>(GetUnderlyingObject(
>           LoadNode->getMemOperand()->getValue(), *getDataLayout()))) {
> 
> -    SDValue Ptr = DAG.getZExtOrTrunc(LoadNode->getBasePtr(), DL,
> -        getPointerTy(AMDGPUAS::PRIVATE_ADDRESS));
> -    Ptr = DAG.getNode(ISD::SRL, DL, MVT::i32, Ptr,
> -        DAG.getConstant(2, DL, MVT::i32));
> -    return DAG.getNode(AMDGPUISD::REGISTER_LOAD, DL, Op->getVTList(),
> -                       LoadNode->getChain(), Ptr,
> -                       DAG.getTargetConstant(0, DL, MVT::i32),
> -                       Op.getOperand(2));
> +      SDValue Ptr = DAG.getZExtOrTrunc(LoadNode->getBasePtr(), DL,
> +          getPointerTy(AMDGPUAS::PRIVATE_ADDRESS));
> +      Ptr = DAG.getNode(ISD::SRL, DL, MVT::i32, Ptr,
> +          DAG.getConstant(2, DL, MVT::i32));
> +      return DAG.getNode(AMDGPUISD::REGISTER_LOAD, DL, Op->getVTList(),
> +                         LoadNode->getChain(), Ptr,
> +                         DAG.getTargetConstant(0, DL, MVT::i32),
> +                         Op.getOperand(2));
> +    } else {
> +      /* Replace with global load from the same address */
C style comment.

> +      EVT MemVT = LoadNode->getMemoryVT();
> +      Type *InitTy = LoadNode->getMemOperand()->getValue()->getType();
> +      PointerType *PtrTy = PointerType::get(InitTy, AMDGPUAS::GLOBAL_ADDRESS);
> +      Ptr = DAG.getZExtOrTrunc(Ptr, DL, getPointerTy(AMDGPUAS::GLOBAL_ADDRESS));
> +      return DAG.getExtLoad(LoadNode->getExtensionType(),
> +                            DL, VT, Chain, Ptr,
> +                            MachinePointerInfo(UndefValue::get(PtrTy)), MemVT,
> +                            LoadNode->isVolatile(),
> +                            LoadNode->isNonTemporal(),
> +                            LoadNode->isInvariant(),
> +                            LoadNode->getAlignment());
> +    }
>   }

I’m not sure you really need to do this. I would rather not replace the underlying address space of the value, since keeping the original address space in the MachineMemOperand can be useful information later. The MachineMemOperand form of the get*Load functions is also shorter and less error prone.


> 
>   if (LoadNode->getAddressSpace() == AMDGPUAS::LOCAL_ADDRESS && VT.isVector()) {
> -- 
> 2.1.0
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <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/20150525/665ed627/attachment.html>