[llvm-commits] [llvm] r149600 - in /llvm/trunk: lib/Target/X86/X86ISelLowering.cpp lib/Target/X86/X86ISelLowering.h lib/Target/X86/X86InstrFragmentsSIMD.td lib/Target/X86/X86InstrSSE.td test/CodeGen/X86/avx-sext.ll
Chandler Carruth
chandlerc at google.com
Thu Feb 2 01:36:20 PST 2012
On Thu, Feb 2, 2012 at 1:10 AM, Elena Demikhovsky <
elena.demikhovsky at intel.com> wrote:
> Author: delena
> Date: Thu Feb 2 03:10:43 2012
> New Revision: 149600
>
> URL: http://llvm.org/viewvc/llvm-project?rev=149600&view=rev
> Log:
> Optimization for SIGN_EXTEND operation on AVX.
> Special handling was added for v4i32 -> v4i64 and v8i16 -> v8i32
> extensions.
>
Why was this submitted without review? Your original patch was only mailed
to the list 19 hours ago.
This patch doesn't adhere to LLVM coding conventions very closely at all.
Please try to make your code look more like the surrounding code. I've
commented a few obvious places below.
+static SDValue PerformSExtCombine(SDNode *N, SelectionDAG &DAG,
> + TargetLowering::DAGCombinerInfo &DCI,
> + const X86Subtarget *Subtarget) {
> + if (!DCI.isBeforeLegalizeOps())
> + return SDValue();
> +
> + if (!Subtarget->hasAVX()) return SDValue();
> +
> + // Optimize vectors in AVX mode
> + // Sign extend v8i16 to v8i32 and
> + // v4i32 to v4i64
> + //
> + // Divide input vector into two parts
> + // for v4i32 the shuffle mask will be { 0, 1, -1, -1} {2, 3, -1, -1}
> + // use vpmovsx instruction to extend v4i32 -> v2i64; v8i16 -> v4i32
> + // concat the vectors to original VT
>
Why is this comment block indented more than the rest of the code?
> +
> + EVT VT = N->getValueType(0);
> + SDValue Op = N->getOperand(0);
> + EVT OpVT = Op.getValueType();
> + DebugLoc dl = N->getDebugLoc();
> +
> + if (((VT == MVT::v4i64) && (OpVT == MVT::v4i32)) ||
> + ((VT == MVT::v8i32) && (OpVT == MVT::v8i16))) {
>
Please don't wrap comparison operators ('==', '>', '<', etc...) in
extraneous parentheses. This defeats many warnings Clang and GCC both
implement which use redundant parentheses to force explicit syntax for
these two constructs:
if ((x = y)) { ... } // Here we assign, and use extra parens to indicate
it was intended.
if (x == y) { ... } // Here we compare, and w/o the parens will get a
warning if we typo '==' as '='.
Also, please line up with the open '('s at each level when breaking a line.
> +
> + unsigned NumElems = OpVT.getVectorNumElements();
> + SmallVector<int,8> ShufMask1(NumElems, -1);
> + for (unsigned i=0; i< NumElems/2; i++) ShufMask1[i] = i;
>
Please use the spacing and whitespace conventional in LLVM for your loops.
spaces on both sides of '=' and '<' are ubiquitous.
> +
> + SDValue OpLo = DAG.getVectorShuffle(OpVT, dl, Op, DAG.getUNDEF(OpVT),
> + ShufMask1.data());
>
Please line up with the '('.
> +
> + SmallVector<int,8> ShufMask2(NumElems, -1);
> + for (unsigned i=0; i< NumElems/2; i++) ShufMask2[i] = i+NumElems/2;
> +
> + SDValue OpHi = DAG.getVectorShuffle(OpVT, dl, Op, DAG.getUNDEF(OpVT),
> + ShufMask2.data());
>
Please line up with the '('.
> +
> + EVT HalfVT = EVT::getVectorVT(*DAG.getContext(), VT.getScalarType(),
> + VT.getVectorNumElements()/2);
>
Please line up with the '('.
> +
> + OpLo = DAG.getNode(X86ISD::VSEXT_MOVL, dl, HalfVT, OpLo);
> + OpHi = DAG.getNode(X86ISD::VSEXT_MOVL, dl, HalfVT, OpHi);
> +
> + return DAG.getNode(ISD::CONCAT_VECTORS, dl, VT, OpLo, OpHi);
> + }
> + return SDValue();
> +}
> +
> static SDValue PerformZExtCombine(SDNode *N, SelectionDAG &DAG,
> const X86Subtarget *Subtarget) {
> // (i32 zext (and (i8 x86isd::setcc_carry), 1)) ->
> @@ -14886,6 +14936,7 @@
> case X86ISD::BT: return PerformBTCombine(N, DAG, DCI);
> case X86ISD::VZEXT_MOVL: return PerformVZEXT_MOVLCombine(N, DAG);
> case ISD::ZERO_EXTEND: return PerformZExtCombine(N, DAG, Subtarget);
> + case ISD::SIGN_EXTEND: return PerformSExtCombine(N, DAG, DCI,
> Subtarget);
> case ISD::TRUNCATE: return PerformTruncateCombine(N, DAG, DCI);
> case X86ISD::SETCC: return PerformSETCCCombine(N, DAG);
> case X86ISD::SHUFP: // Handle all target specific shuffles
>
> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.h?rev=149600&r1=149599&r2=149600&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86ISelLowering.h (original)
> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.h Thu Feb 2 03:10:43 2012
> @@ -219,6 +219,9 @@
> // VZEXT_MOVL - Vector move low and zero extend.
> VZEXT_MOVL,
>
> + // VZEXT_MOVL - Vector move low and sign extend.
>
This comment is wrong.
> + VSEXT_MOVL,
> +
> // VSHL, VSRL - 128-bit vector logical left / right shift
> VSHLDQ, VSRLDQ,
>
>
> Modified: llvm/trunk/lib/Target/X86/X86InstrFragmentsSIMD.td
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrFragmentsSIMD.td?rev=149600&r1=149599&r2=149600&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86InstrFragmentsSIMD.td (original)
> +++ llvm/trunk/lib/Target/X86/X86InstrFragmentsSIMD.td Thu Feb 2 03:10:43
> 2012
> @@ -71,6 +71,9 @@
> SDTCisVT<2, v4f32>,
> SDTCisPtrTy<3>]>>;
> def X86vzmovl : SDNode<"X86ISD::VZEXT_MOVL",
> SDTypeProfile<1, 1, [SDTCisSameAs<0,1>]>>;
> +def X86vsmovl : SDNode<"X86ISD::VSEXT_MOVL",
> + SDTypeProfile<1, 1, [SDTCisVec<0>, SDTCisInt<1>,
> SDTCisInt<0>]>>;
> +
> def X86vzload : SDNode<"X86ISD::VZEXT_LOAD", SDTLoad,
> [SDNPHasChain, SDNPMayLoad, SDNPMemOperand]>;
> def X86vshldq : SDNode<"X86ISD::VSHLDQ", SDTIntShiftOp>;
>
> Modified: llvm/trunk/lib/Target/X86/X86InstrSSE.td
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrSSE.td?rev=149600&r1=149599&r2=149600&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86InstrSSE.td (original)
> +++ llvm/trunk/lib/Target/X86/X86InstrSSE.td Thu Feb 2 03:10:43 2012
> @@ -5478,6 +5478,16 @@
> (PMOVZXDQrm addr:$src)>;
> }
>
> +let Predicates = [HasAVX] in {
> +def : Pat<(v2i64 (X86vsmovl (v4i32 VR128:$src))), (VPMOVSXDQrr
> VR128:$src)>;
> +def : Pat<(v4i32 (X86vsmovl (v8i16 VR128:$src))), (VPMOVSXWDrr
> VR128:$src)>;
> +}
> +
> +let Predicates = [HasSSE41] in {
> +def : Pat<(v2i64 (X86vsmovl (v4i32 VR128:$src))), (PMOVSXDQrr
> VR128:$src)>;
> +def : Pat<(v4i32 (X86vsmovl (v8i16 VR128:$src))), (PMOVSXWDrr
> VR128:$src)>;
> +}
> +
>
> multiclass SS41I_binop_rm_int4<bits<8> opc, string OpcodeStr, Intrinsic
> IntId> {
> def rr : SS48I<opc, MRMSrcReg, (outs VR128:$dst), (ins VR128:$src),
>
> Added: llvm/trunk/test/CodeGen/X86/avx-sext.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/avx-sext.ll?rev=149600&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/avx-sext.ll (added)
> +++ llvm/trunk/test/CodeGen/X86/avx-sext.ll Thu Feb 2 03:10:43 2012
> @@ -0,0 +1,17 @@
> +; RUN: llc < %s -mtriple=x86_64-apple-darwin -mcpu=corei7-avx -mattr=+avx
> | FileCheck %s
> +
> +define <8 x i32> @sext_8i16_to_8i32(<8 x i16> %A) nounwind uwtable
> readnone ssp {
> +;CHECK: sext_8i16_to_8i32
> +;CHECK: vpmovsxwd
> +
> + %B = sext <8 x i16> %A to <8 x i32>
> + ret <8 x i32>%B
> +}
> +
> +define <4 x i64> @sext_4i32_to_4i64(<4 x i32> %A) nounwind uwtable
> readnone ssp {
> +;CHECK: sext_4i32_to_4i64
> +;CHECK: vpmovsxdq
> +
> + %B = sext <4 x i32> %A to <4 x i64>
> + ret <4 x i64>%B
> +}
>
> Propchange: llvm/trunk/test/CodeGen/X86/avx-sext.ll
>
> ------------------------------------------------------------------------------
> svn:executable = *
>
>
> _______________________________________________
> 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/20120202/4f7495a5/attachment.html>
More information about the llvm-commits
mailing list