[llvm] r177477 - Mark all variable shifts needing customizing

Michael Liao michael.liao at intel.com
Wed Mar 20 15:02:23 PDT 2013


committed as r177586. - michael

On Wed, 2013-03-20 at 13:53 -0700, Nadav Rotem wrote:
> LGTM. 
> 
> On Mar 20, 2013, at 1:49 PM, Michael Liao <michael.liao at intel.com>
> wrote:
> 
> > 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
> > > 
> > > 
> > 
> > <0001-Correct-cost-model-for-vector-shift-on-AVX2.patch>
> 
> 





More information about the llvm-commits mailing list