[llvm-commits] [llvm] r168883 - in /llvm/trunk: lib/CodeGen/SelectionDAG/LegalizeTypes.h lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp test/CodeGen/NVPTX/vector-select.ll

[Nadav Rotem]

Hi Justin, 

I have a few minor comments below.

> 
> +  SDValue Lo, Hi;
> +  GetSplitVector(N->getOperand(0), Lo, Hi);

Can you assert that Lo and Hi have the same type ?

> +  unsigned LoNumElts = Lo.getValueType().getVectorNumElements();
> +  unsigned HiNumElts = Hi.getValueType().getVectorNumElements();
> +  assert(LoNumElts == HiNumElts && "Asymmetric vector split?");
> +
> +  EVT LoOpVT = EVT::getVectorVT(*DAG.getContext(),
> +                                Src0.getValueType().getVectorElementType(),
> +                                LoNumElts);

Can you save the context once and use it in all of the functions below ?  Can you save the type of Src0 and MaskVT number of elements ? These changes will allow you to put most of the code below in one line (each), which will make it easier to review. 

> +  EVT LoMaskVT = EVT::getVectorVT(*DAG.getContext(),
> +                                  MaskVT.getVectorElementType(),
> +                                  LoNumElts);
> +  EVT HiOpVT = EVT::getVectorVT(*DAG.getContext(),
> +                                Src0.getValueType().getVectorElementType(),
> +                                HiNumElts);
> +  EVT HiMaskVT = EVT::getVectorVT(*DAG.getContext(),
> +                                  MaskVT.getVectorElementType(),
> +                                  HiNumElts);
> +
> +  SDValue LoOp0 = DAG.getNode(ISD::EXTRACT_SUBVECTOR, DL, LoOpVT, Src0,
> +                              DAG.getIntPtrConstant(0));
> +  SDValue LoOp1 = DAG.getNode(ISD::EXTRACT_SUBVECTOR, DL, LoOpVT, Src1,
> +                              DAG.getIntPtrConstant(0));
> +

Same for the Zero constant, and for the number of elements. 

> +  SDValue HiOp0 = DAG.getNode(ISD::EXTRACT_SUBVECTOR, DL, HiOpVT, Src0,
> +                              DAG.getIntPtrConstant(LoNumElts));
> +  SDValue HiOp1 = DAG.getNode(ISD::EXTRACT_SUBVECTOR, DL, HiOpVT, Src1,
> +                              DAG.getIntPtrConstant(LoNumElts));
> +
> +  SDValue LoMask = DAG.getNode(ISD::EXTRACT_SUBVECTOR, DL, LoMaskVT, Mask,
> +                               DAG.getIntPtrConstant(0));
> +  SDValue HiMask = DAG.getNode(ISD::EXTRACT_SUBVECTOR, DL, HiMaskVT, Mask,
> +                               DAG.getIntPtrConstant(LoNumElts));
> +


Thanks,
Nadav