[llvm] r177477 - Mark all variable shifts needing customizing

Michael Liao michael.liao at intel.com
Wed Mar 20 13:49:47 PDT 2013


Hi Nadav

Please review the attached patch fixing cost model for AVX2 vector
shifts. Test cases is added to keep cost consistent with the one before
this patch.

Yours
- Michael

On Tue, 2013-03-19 at 21:21 -0700, Nadav Rotem wrote:
> Michael, 
> 
> 
> The TargetTransformInfo depends on the OpActions table for determining
> the costs of instructions. It assumes that the cost of 'legal'
> operation is lower than the cost of custom operations. When we mark
> everything as custom then TTI has no way of estimating the cost of
> operations without using the cost tables. I understand that your
> change is needed in order to move logic from DAGCombine to the
> Lowering phase, and generally this is a good change.  Can you please
> take a look at the cost tables and make sure that we did not miss
> anything due to this change?
> 
> 
> Thanks,
> Nadav
>  
> 
> 
> 
> On Mar 19, 2013, at 7:28 PM, Michael Liao <michael.liao at intel.com>
> wrote:
> 
> > Author: hliao
> > Date: Tue Mar 19 21:28:20 2013
> > New Revision: 177477
> > 
> > URL: http://llvm.org/viewvc/llvm-project?rev=177477&view=rev
> > Log:
> > Mark all variable shifts needing customizing
> > 
> > - Prepare moving logic from DAG combining into DAG lowering. There's
> > no
> >  functionality change.
> > 
> > 
> > Modified:
> >    llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> > 
> > Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=177477&r1=177476&r2=177477&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> > +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Tue Mar 19
> > 21:28:20 2013
> > @@ -1053,23 +1053,16 @@ X86TargetLowering::X86TargetLowering(X86
> >     setOperationAction(ISD::SRA,               MVT::v8i16, Custom);
> >     setOperationAction(ISD::SRA,               MVT::v16i8, Custom);
> > 
> > -    if (Subtarget->hasInt256()) {
> > -      setOperationAction(ISD::SRL,             MVT::v2i64, Legal);
> > -      setOperationAction(ISD::SRL,             MVT::v4i32, Legal);
> > -
> > -      setOperationAction(ISD::SHL,             MVT::v2i64, Legal);
> > -      setOperationAction(ISD::SHL,             MVT::v4i32, Legal);
> > +    // In the customized shift lowering, the legal cases in AVX2
> > will be
> > +    // recognized.
> > +    setOperationAction(ISD::SRL,               MVT::v2i64, Custom);
> > +    setOperationAction(ISD::SRL,               MVT::v4i32, Custom);
> > 
> > -      setOperationAction(ISD::SRA,             MVT::v4i32, Legal);
> > -    } else {
> > -      setOperationAction(ISD::SRL,             MVT::v2i64, Custom);
> > -      setOperationAction(ISD::SRL,             MVT::v4i32, Custom);
> > +    setOperationAction(ISD::SHL,               MVT::v2i64, Custom);
> > +    setOperationAction(ISD::SHL,               MVT::v4i32, Custom);
> > 
> > -      setOperationAction(ISD::SHL,             MVT::v2i64, Custom);
> > -      setOperationAction(ISD::SHL,             MVT::v4i32, Custom);
> > +    setOperationAction(ISD::SRA,               MVT::v4i32, Custom);
> > 
> > -      setOperationAction(ISD::SRA,             MVT::v4i32, Custom);
> > -    }
> >     setOperationAction(ISD::SDIV,              MVT::v8i16, Custom);
> >     setOperationAction(ISD::SDIV,              MVT::v4i32, Custom);
> >   }
> > @@ -1186,14 +1179,6 @@ X86TargetLowering::X86TargetLowering(X86
> > 
> >       setOperationAction(ISD::VSELECT,         MVT::v32i8, Legal);
> > 
> > -      setOperationAction(ISD::SRL,             MVT::v4i64, Legal);
> > -      setOperationAction(ISD::SRL,             MVT::v8i32, Legal);
> > -
> > -      setOperationAction(ISD::SHL,             MVT::v4i64, Legal);
> > -      setOperationAction(ISD::SHL,             MVT::v8i32, Legal);
> > -
> > -      setOperationAction(ISD::SRA,             MVT::v8i32, Legal);
> > -
> >       setOperationAction(ISD::SDIV,            MVT::v8i32, Custom);
> >     } else {
> >       setOperationAction(ISD::ADD,             MVT::v4i64, Custom);
> > @@ -1210,15 +1195,17 @@ X86TargetLowering::X86TargetLowering(X86
> >       setOperationAction(ISD::MUL,             MVT::v8i32, Custom);
> >       setOperationAction(ISD::MUL,             MVT::v16i16, Custom);
> >       // Don't lower v32i8 because there is no 128-bit byte mul
> > +    }
> > 
> > -      setOperationAction(ISD::SRL,             MVT::v4i64, Custom);
> > -      setOperationAction(ISD::SRL,             MVT::v8i32, Custom);
> > +    // In the customized shift lowering, the legal cases in AVX2
> > will be
> > +    // recognized.
> > +    setOperationAction(ISD::SRL,               MVT::v4i64, Custom);
> > +    setOperationAction(ISD::SRL,               MVT::v8i32, Custom);
> > 
> > -      setOperationAction(ISD::SHL,             MVT::v4i64, Custom);
> > -      setOperationAction(ISD::SHL,             MVT::v8i32, Custom);
> > +    setOperationAction(ISD::SHL,               MVT::v4i64, Custom);
> > +    setOperationAction(ISD::SHL,               MVT::v8i32, Custom);
> > 
> > -      setOperationAction(ISD::SRA,             MVT::v8i32, Custom);
> > -    }
> > +    setOperationAction(ISD::SRA,               MVT::v8i32, Custom);
> > 
> >     // Custom lower several nodes for 256-bit types.
> >     for (int i = MVT::FIRST_VECTOR_VALUETYPE;
> > @@ -11626,6 +11613,20 @@ SDValue X86TargetLowering::LowerShift(SD
> >   if (V.getNode())
> >     return V;
> > 
> > +  // AVX2 has VPSLLV/VPSRAV/VPSRLV.
> > +  if (Subtarget->hasInt256()) {
> > +    if (Op.getOpcode() == ISD::SRL &&
> > +        (VT == MVT::v2i64 || VT == MVT::v4i32 ||
> > +         VT == MVT::v4i64 || VT == MVT::v8i32))
> > +      return Op;
> > +    if (Op.getOpcode() == ISD::SHL &&
> > +        (VT == MVT::v2i64 || VT == MVT::v4i32 ||
> > +         VT == MVT::v4i64 || VT == MVT::v8i32))
> > +      return Op;
> > +    if (Op.getOpcode() == ISD::SRA && (VT == MVT::v4i32 || VT ==
> > MVT::v8i32))
> > +      return Op;
> > +  }
> > +
> >   // Lower SHL with variable shift amount.
> >   if (VT == MVT::v4i32 && Op->getOpcode() == ISD::SHL) {
> >     Op = DAG.getNode(ISD::SHL, dl, VT, Amt, DAG.getConstant(23,
> > VT));
> > 
> > 
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Correct-cost-model-for-vector-shift-on-AVX2.patch
Type: text/x-patch
Size: 3875 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130320/fd544462/attachment.bin>


More information about the llvm-commits mailing list