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

[Justin Holewinski]

Done.  Committed in r168932.

> -----Original Message-----
> From: Nadav Rotem [mailto:nrotem at apple.com]
> Sent: Thursday, November 29, 2012 12:38 PM
> To: Justin Holewinski
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [llvm] r168883 - in /llvm/trunk:
> lib/CodeGen/SelectionDAG/LegalizeTypes.h
> lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp test/CodeGen/NVPTX/vector-
> select.ll
> 
> 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

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------