[llvm] r194542 - SelectionDAG: Teach the legalizer to split SETCC if VSELECT needs splitting too.
Juergen Ributzka
juergen at apple.com
Fri Nov 15 10:45:11 PST 2013
Hi Tom,
I took a quick look yesterday night and I didn’t see a quick fix either. I will look into it today.
Cheers,
Juergen
On Nov 14, 2013, at 8:45 PM, Tom Stellard <tom at stellard.net> wrote:
> 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
> <0001-R600-Fix-getSetCCResultType-to-handle-new-VSELECT-DA.patch>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131115/fee07b5a/attachment.html>
More information about the llvm-commits
mailing list