[llvm] r194542 - SelectionDAG: Teach the legalizer to split SETCC if VSELECT needs splitting too.

Tom Stellard tom at stellard.net
Thu Nov 14 20:45:12 PST 2013


On Wed, Nov 13, 2013 at 01:57:54AM -0000, Juergen Ributzka wrote:
> Author: ributzka
> Date: Tue Nov 12 19:57:54 2013
> New Revision: 194542
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=194542&view=rev
> Log:
> SelectionDAG: Teach the legalizer to split SETCC if VSELECT needs splitting too.
> 
> This patch reapplies r193676 with an additional fix for the Hexagon backend. The
> SystemZ backend has already been fixed by r194148.
> 
> The Type Legalizer recognizes that VSELECT needs to be split, because the type
> is to wide for the given target. The same does not always apply to SETCC,
> because less space is required to encode the result of a comparison. As a result
> VSELECT is split and SETCC is unrolled into scalar comparisons.
> 
> This commit fixes the issue by checking for VSELECT-SETCC patterns in the DAG
> Combiner. If a matching pattern is found, then the result mask of SETCC is
> promoted to the expected vector mask type for the given target. Now the type
> legalizer will split both VSELECT and SETCC.
> 
> This allows the following X86 DAG Combine code to sucessfully detect the MIN/MAX
> pattern. This fixes PR16695, PR17002, and <rdar://problem/14594431>.
> 
> Reviewed by Nadav
>

Hi Juergen,

This caused a regression on R600.  I have attached a patch to this email
with a testcase.  See my comments below.

> Added:
>     llvm/trunk/test/CodeGen/X86/vec_split.ll
> Modified:
>     llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>     llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp
>     llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.h
>     llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> 
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=194542&r1=194541&r2=194542&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Tue Nov 12 19:57:54 2013
> @@ -4364,6 +4364,29 @@ SDValue DAGCombiner::visitVSELECT(SDNode
>      }
>    }
>  
> +  // Treat SETCC as a vector mask and promote the result type based on the
> +  // targets expected SETCC result type. This will ensure that SETCC and VSELECT
> +  // are both split by the type legalizer. This is done to prevent the type
> +  // legalizer from unrolling SETCC into scalar comparions.
> +  EVT SelectVT = N->getValueType(0);
> +  EVT MaskVT = getSetCCResultType(SelectVT);
> +  assert(MaskVT.isVector() && "Expected a vector type.");
> +  if (N0.getOpcode() == ISD::SETCC && N0.getValueType() != MaskVT) {
> +    SDLoc MaskDL(N0);
> +
> +    // Extend the mask to the desired value type.
> +    ISD::NodeType ExtendCode =
> +      TargetLowering::getExtendForContent(TLI.getBooleanContents(true));
> +    SDValue Mask = DAG.getNode(ExtendCode, MaskDL, MaskVT, N0);
> +
> +    AddToWorkList(Mask.getNode());
> +
> +    SDValue LHS = N->getOperand(1);
> +    SDValue RHS = N->getOperand(2);
> +
> +    return DAG.getNode(ISD::VSELECT, DL, SelectVT, Mask, LHS, RHS);
> +  }
> +
>    return SDValue();
>  }
>  
> 
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp?rev=194542&r1=194541&r2=194542&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp Tue Nov 12 19:57:54 2013
> @@ -492,14 +492,19 @@ void DAGTypeLegalizer::SplitRes_SELECT(S
>    SDValue Cond = N->getOperand(0);
>    CL = CH = Cond;
>    if (Cond.getValueType().isVector()) {
> -    assert(Cond.getValueType().getVectorElementType() == MVT::i1 &&
> -           "Condition legalized before result?");
> -    unsigned NumElements = Cond.getValueType().getVectorNumElements();
> -    EVT VCondTy = EVT::getVectorVT(*DAG.getContext(), MVT::i1, NumElements / 2);
> -    CL = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, VCondTy, Cond,
> -                     DAG.getConstant(0, TLI.getVectorIdxTy()));
> -    CH = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, VCondTy, Cond,
> -                     DAG.getConstant(NumElements / 2, TLI.getVectorIdxTy()));
> +    if (Cond.getOpcode() == ISD::SETCC) {
> +      assert(Cond.getValueType() == getSetCCResultType(N->getValueType(0)) &&
> +             "Condition has not been prepared for split!");
> +      GetSplitVector(Cond, CL, CH);

Even with the adjustment to getSetCCResultType in the attached patch,
when GetSplitVector() is called here, I am getting an assertion failure:

llc: LegalizeTypes.cpp:828: void llvm::DAGTypeLegalizer::GetSplitVector(llvm::SDValue, llvm::SDValue &, llvm::SDValue &): Assertion `Entry.first.getNode() && "Operand isn't split"' failed.

>From what I can tell the problem here is that 'Cond' has a value type
of v4i32 which is legal on R600, so the operation was not split, and
therefore there is no entry in the SplitVectors map for it (This is what
is causing the assertion failure).

The SELECT in this example has a type of v4i64 which is not a legal type
on R600, so the problem occurs when a SELECT node has an illegal type,
but Operand 0 is a SETCC node with a legal type.  I'm not sure the best
way to fix this issue.  Do you have any suggestions?

Thanks,
Tom


> +    } else {
> +      EVT ETy = Cond.getValueType().getVectorElementType();
> +      unsigned NumElements = Cond.getValueType().getVectorNumElements();
> +      EVT VCondTy = EVT::getVectorVT(*DAG.getContext(), ETy, NumElements / 2);
> +      CL = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, VCondTy, Cond,
> +                       DAG.getConstant(0, TLI.getVectorIdxTy()));
> +      CH = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, VCondTy, Cond,
> +                       DAG.getConstant(NumElements / 2, TLI.getVectorIdxTy()));
> +    }
>    }
>  
>    Lo = DAG.getNode(N->getOpcode(), dl, LL.getValueType(), CL, LL, RL);
> 
> Modified: llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.h?rev=194542&r1=194541&r2=194542&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.h (original)
> +++ llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.h Tue Nov 12 19:57:54 2013
> @@ -141,8 +141,11 @@ namespace llvm {
>  
>      SDValue  LowerVASTART(SDValue Op, SelectionDAG &DAG) const;
>      SDValue  LowerConstantPool(SDValue Op, SelectionDAG &DAG) const;
> -    virtual EVT getSetCCResultType(LLVMContext &, EVT) const {
> -      return MVT::i1;
> +    virtual EVT getSetCCResultType(LLVMContext &C, EVT VT) const {
> +      if (!VT.isVector())
> +        return MVT::i1;
> +      else
> +        return EVT::getVectorVT(C, MVT::i1, VT.getVectorNumElements());
>      }
>  
>      virtual bool getPostIndexedAddressParts(SDNode *N, SDNode *Op,
> 
> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=194542&r1=194541&r2=194542&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Tue Nov 12 19:57:54 2013
> @@ -1547,7 +1547,16 @@ void X86TargetLowering::resetOperationAc
>  }
>  
>  EVT X86TargetLowering::getSetCCResultType(LLVMContext &, EVT VT) const {
> -  if (!VT.isVector()) return MVT::i8;
> +  if (!VT.isVector())
> +    return MVT::i8;
> +
> +  const TargetMachine &TM = getTargetMachine();
> +  if (!TM.Options.UseSoftFloat && Subtarget->hasAVX512())
> +    switch(VT.getVectorNumElements()) {
> +    case  8: return MVT::v8i1;
> +    case 16: return MVT::v16i1;
> +    }
> +
>    return VT.changeVectorElementTypeToInteger();
>  }
>  
> 
> Added: llvm/trunk/test/CodeGen/X86/vec_split.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/vec_split.ll?rev=194542&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/vec_split.ll (added)
> +++ llvm/trunk/test/CodeGen/X86/vec_split.ll Tue Nov 12 19:57:54 2013
> @@ -0,0 +1,42 @@
> +; RUN: llc -march=x86-64 -mcpu=corei7 < %s | FileCheck %s -check-prefix=SSE4
> +; RUN: llc -march=x86-64 -mcpu=corei7-avx < %s | FileCheck %s -check-prefix=AVX1
> +; RUN: llc -march=x86-64 -mcpu=core-avx2 < %s | FileCheck %s -check-prefix=AVX2
> +
> +define <16 x i16> @split16(<16 x i16> %a, <16 x i16> %b, <16 x i8> %__mask) {
> +; SSE4-LABEL: split16:
> +; SSE4: pminuw
> +; SSE4: pminuw
> +; SSE4: ret
> +; AVX1-LABEL: split16:
> +; AVX1: vpminuw
> +; AVX1: vpminuw
> +; AVX1: ret
> +; AVX2-LABEL: split16:
> +; AVX2: vpminuw
> +; AVX2: ret
> +  %1 = icmp ult <16 x i16> %a, %b
> +  %2 = select <16 x i1> %1, <16 x i16> %a, <16 x i16> %b
> +  ret <16 x i16> %2
> +}
> +
> +define <32 x i16> @split32(<32 x i16> %a, <32 x i16> %b, <32 x i8> %__mask) {
> +; SSE4-LABEL: split32:
> +; SSE4: pminuw
> +; SSE4: pminuw
> +; SSE4: pminuw
> +; SSE4: pminuw
> +; SSE4: ret
> +; AVX1-LABEL: split32:
> +; AVX1: vpminuw
> +; AVX1: vpminuw
> +; AVX1: vpminuw
> +; AVX1: vpminuw
> +; AVX1: ret
> +; AVX2-LABEL: split32:
> +; AVX2: vpminuw
> +; AVX2: vpminuw
> +; AVX2: ret
> +  %1 = icmp ult <32 x i16> %a, %b
> +  %2 = select <32 x i1> %1, <32 x i16> %a, <32 x i16> %b
> +  ret <32 x i16> %2
> +}
> 
> 
> _______________________________________________
> 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-R600-Fix-getSetCCResultType-to-handle-new-VSELECT-DA.patch
Type: text/x-diff
Size: 1789 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131114/aa036529/attachment.patch>


More information about the llvm-commits mailing list