<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Please let me know when you have the patches in ToT. I can merge them into the 3.4 branch afterwards.<div><br></div><div>-bw</div><div><br><div style=""><div>On Nov 20, 2013, at 11:30 PM, Owen Anderson <<a href="mailto:resistor@mac.com">resistor@mac.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">I'm fine with both of these patches.<div><br></div><div>--Owen</div><div><br><div><div>On Nov 20, 2013, at 7:35 PM, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Tue, Nov 19, 2013 at 06:40:26PM -0800, Juergen Ributzka wrote:<br><blockquote type="cite">Hi Tom<br><br>Great, that makes things much easier. I rewrote the patch and it shouldn’t require any changes in the backend.<br>Please let me know if this patch works for you.<br></blockquote><br>cc'ing Owen, because I would like to get these patches into the 3.4<br>branch.<br><br>Juergen,<br><br>Your patch does fix the crash, but there is still a performance<br>regression in the test case I provided. I have attached a patch to fix<br>this performance regression.<br><br>Owen,<br><br>There are two patches attached to this email that I would like to merge<br>into the 3.4 branch. The first is a patch from Juergen which fixes a<br>crash in the R600 backend. The crash is a regression caused by r194542.<br><br>Even with the crash fixed there is still a performance regression which<br>was uncovered by r194542. This performance issue is caused by the<br>DAGLegalizer falling back to stack loads and stores when trying to<br>expand certain kinds of bitcasts. The second patch improves the<br>handling of bitcasts in the legalizer and fixes this regression.<br><br>Thanks,<br>Tom<br><br><br><blockquote type="cite"><br>Thanks<br><br>Cheers,<br>Juergen<br><br><br></blockquote><br><br><blockquote type="cite"><br>On Nov 19, 2013, at 3:31 PM, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:<br><br><blockquote type="cite">Hi Juergen,<br><br>Sorry the test case is only meant for VLIW chips, it should work with<br>these arguments to llc:<br><br>llc -march=r600 -mcpu=redwood vselect.ll<br><br>-Tom<br><br><br>On Tue, Nov 19, 2013 at 03:16:08PM -0800, Juergen Ributzka wrote:<br><blockquote type="cite">Hi Tom,<br><br>even after disabling my changes I get the following error from your new test case during instruction selection:<br><br>LLVM ERROR: Cannot select: 0x7ffc34825a10: i64 = select 0x7ffc34825910, 0x7ffc3480f510, 0x7ffc3480e210 [ORD=26] [ID=49]<br>0x7ffc34825910: i1 = setcc 0x7ffc34825810, 0x7ffc34825710, 0x7ffc3480db10 [ORD=25] [ID=45]<br> 0x7ffc34825810: i32 = extract_vector_elt 0x7ffc34049810, 0x7ffc34049510 [ORD=25] [ID=41]<br> 0x7ffc34049810: v2i32 = BUILD_VECTOR 0x7ffc34810c10, 0x7ffc3480e610 [ORD=25] [ID=37]<br> 0x7ffc34810c10: i32 = extract_vector_elt 0x7ffc3480f110, 0x7ffc34049510 [ORD=24] [ID=32]<br> 0x7ffc3480f110: v4i32,ch = load 0x7ffc33c160d8, 0x7ffc34046910, 0x7ffc34810210<LD16[undef]> [ORD=24] [ID=27]<br> 0x7ffc34046910: i64 = add 0x7ffc34047310, 0x7ffc3480f410 [ORD=24] [ID=23]<br> 0x7ffc34047310: i64,ch = CopyFromReg 0x7ffc33c160d8, 0x7ffc34049910 [ORD=24] [ID=21]<br> 0x7ffc34049910: i64 = Register %vreg0 [ID=1]<br> 0x7ffc3480f410: i64 = Constant<52> [ID=4]<br> 0x7ffc34810210: i64 = undef [ID=3]<br> 0x7ffc34049510: i32 = Constant<0> [ID=5]<br> 0x7ffc3480e610: i32 = extract_vector_elt 0x7ffc3480f110, 0x7ffc34049a10 [ORD=24] [ID=31]<br> 0x7ffc3480f110: v4i32,ch = load 0x7ffc33c160d8, 0x7ffc34046910, 0x7ffc34810210<LD16[undef]> [ORD=24] [ID=27]<br> 0x7ffc34046910: i64 = add 0x7ffc34047310, 0x7ffc3480f410 [ORD=24] [ID=23]<br> 0x7ffc34047310: i64,ch = CopyFromReg 0x7ffc33c160d8, 0x7ffc34049910 [ORD=24] [ID=21]<br> 0x7ffc34049910: i64 = Register %vreg0 [ID=1]<br> 0x7ffc3480f410: i64 = Constant<52> [ID=4]<br> 0x7ffc34810210: i64 = undef [ID=3]<br> 0x7ffc34049a10: i32 = Constant<1> [ID=17]<br> 0x7ffc34049510: i32 = Constant<0> [ID=5]<br> 0x7ffc34825710: i32 = extract_vector_elt 0x7ffc3480f310, 0x7ffc34049510 [ORD=25] [ID=26]<br> 0x7ffc3480f310: v2i32 = BUILD_VECTOR 0x7ffc34049510, 0x7ffc34049510 [ORD=25] [ID=22]<br> 0x7ffc34049510: i32 = Constant<0> [ID=5]<br> 0x7ffc34049510: i32 = Constant<0> [ID=5]<br> 0x7ffc34049510: i32 = Constant<0> [ID=5]<br>0x7ffc3480f510: i64 = Constant<0> [ID=7]<br>0x7ffc3480e210: i64 = Constant<4> [ID=11]<br>In function: test_select_v4i64<br><br>Do you get the same result?<br><br>-Juergen<br><br>On Nov 18, 2013, at 5:51 PM, Juergen Ributzka <<a href="mailto:juergen@apple.com">juergen@apple.com</a>> wrote:<br><br><blockquote type="cite">Hi Tom,<br><br>sorry for not coming back to you earlier. I was working on a patch for this problem this weekend, but then I stumbled across another issue that was exposed while working on the patch. The two pending patches for SelectionDAG that I posted to the mailing list are in preparation for fixing this issue. I looked at your second patch and using SplitVecRes_SETCC would fix the problem, but it also splits the SETCC result vector again, instead of using the already split vectors provided by GetSplitVector. GetSplitVector is just a cache of already split vectors for that given operation. I would prefer not to duplicate split vector nodes if they already exist. The issue for your backend is that split never occurred and that is why the assertion is failing. I hope to get this fixed for you soon.<br><br>Cheers,<br>Juergen<br><br>On Nov 18, 2013, at 4:49 PM, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:<br><br><blockquote type="cite">Hi Juergen,<br><br>These two patches fix the crash for me. The first one is an R600 fix<br>and the second is a fix for illegal SELECT with legal SETCC as its<br>condition. I'm not so sure patch number 2 is the right way to<br>fix the bug, but it was the most simple fix I could come up with.<br><br>-Tom<br><br><br>On Fri, Nov 15, 2013 at 10:45:11AM -0800, Juergen Ributzka wrote:<br><blockquote type="cite">Hi Tom,<br><br>I took a quick look yesterday night and I didn?t see a quick fix either. I will look into it today.<br><br>Cheers,<br>Juergen<br><br><br>On Nov 14, 2013, at 8:45 PM, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:<br><br><blockquote type="cite">On Wed, Nov 13, 2013 at 01:57:54AM -0000, Juergen Ributzka wrote:<br><blockquote type="cite">Author: ributzka<br>Date: Tue Nov 12 19:57:54 2013<br>New Revision: 194542<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=194542&view=rev">http://llvm.org/viewvc/llvm-project?rev=194542&view=rev</a><br>Log:<br>SelectionDAG: Teach the legalizer to split SETCC if VSELECT needs splitting too.<br><br>This patch reapplies r193676 with an additional fix for the Hexagon backend. The<br>SystemZ backend has already been fixed by r194148.<br><br>The Type Legalizer recognizes that VSELECT needs to be split, because the type<br>is to wide for the given target. The same does not always apply to SETCC,<br>because less space is required to encode the result of a comparison. As a result<br>VSELECT is split and SETCC is unrolled into scalar comparisons.<br><br>This commit fixes the issue by checking for VSELECT-SETCC patterns in the DAG<br>Combiner. If a matching pattern is found, then the result mask of SETCC is<br>promoted to the expected vector mask type for the given target. Now the type<br>legalizer will split both VSELECT and SETCC.<br><br>This allows the following X86 DAG Combine code to sucessfully detect the MIN/MAX<br>pattern. This fixes PR16695, PR17002, and <<a href="rdar://problem/14594431">rdar://problem/14594431</a>>.<br><br>Reviewed by Nadav<br><br></blockquote><br>Hi Juergen,<br><br>This caused a regression on R600. I have attached a patch to this email<br>with a testcase. See my comments below.<br><br><blockquote type="cite">Added:<br>llvm/trunk/test/CodeGen/X86/vec_split.ll<br>Modified:<br>llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp<br>llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.h<br>llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br><br>Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=194542&r1=194541&r2=194542&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=194542&r1=194541&r2=194542&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)<br>+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Tue Nov 12 19:57:54 2013<br>@@ -4364,6 +4364,29 @@ SDValue DAGCombiner::visitVSELECT(SDNode<br>}<br>}<br><br>+ // Treat SETCC as a vector mask and promote the result type based on the<br>+ // targets expected SETCC result type. This will ensure that SETCC and VSELECT<br>+ // are both split by the type legalizer. This is done to prevent the type<br>+ // legalizer from unrolling SETCC into scalar comparions.<br>+ EVT SelectVT = N->getValueType(0);<br>+ EVT MaskVT = getSetCCResultType(SelectVT);<br>+ assert(MaskVT.isVector() && "Expected a vector type.");<br>+ if (N0.getOpcode() == ISD::SETCC && N0.getValueType() != MaskVT) {<br>+ SDLoc MaskDL(N0);<br>+<br>+ // Extend the mask to the desired value type.<br>+ ISD::NodeType ExtendCode =<br>+ TargetLowering::getExtendForContent(TLI.getBooleanContents(true));<br>+ SDValue Mask = DAG.getNode(ExtendCode, MaskDL, MaskVT, N0);<br>+<br>+ AddToWorkList(Mask.getNode());<br>+<br>+ SDValue LHS = N->getOperand(1);<br>+ SDValue RHS = N->getOperand(2);<br>+<br>+ return DAG.getNode(ISD::VSELECT, DL, SelectVT, Mask, LHS, RHS);<br>+ }<br>+<br>return SDValue();<br>}<br><br><br>Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp?rev=194542&r1=194541&r2=194542&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp?rev=194542&r1=194541&r2=194542&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp (original)<br>+++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp Tue Nov 12 19:57:54 2013<br>@@ -492,14 +492,19 @@ void DAGTypeLegalizer::SplitRes_SELECT(S<br>SDValue Cond = N->getOperand(0);<br>CL = CH = Cond;<br>if (Cond.getValueType().isVector()) {<br>- assert(Cond.getValueType().getVectorElementType() == MVT::i1 &&<br>- "Condition legalized before result?");<br>- unsigned NumElements = Cond.getValueType().getVectorNumElements();<br>- EVT VCondTy = EVT::getVectorVT(*DAG.getContext(), MVT::i1, NumElements / 2);<br>- CL = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, VCondTy, Cond,<br>- DAG.getConstant(0, TLI.getVectorIdxTy()));<br>- CH = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, VCondTy, Cond,<br>- DAG.getConstant(NumElements / 2, TLI.getVectorIdxTy()));<br>+ if (Cond.getOpcode() == ISD::SETCC) {<br>+ assert(Cond.getValueType() == getSetCCResultType(N->getValueType(0)) &&<br>+ "Condition has not been prepared for split!");<br>+ GetSplitVector(Cond, CL, CH);<br></blockquote><br>Even with the adjustment to getSetCCResultType in the attached patch,<br>when GetSplitVector() is called here, I am getting an assertion failure:<br><br>llc: LegalizeTypes.cpp:828: void llvm::DAGTypeLegalizer::GetSplitVector(llvm::SDValue, llvm::SDValue &, llvm::SDValue &): Assertion `Entry.first.getNode() && "Operand isn't split"' failed.<br><br>From what I can tell the problem here is that 'Cond' has a value type<br>of v4i32 which is legal on R600, so the operation was not split, and<br>therefore there is no entry in the SplitVectors map for it (This is what<br>is causing the assertion failure).<br><br>The SELECT in this example has a type of v4i64 which is not a legal type<br>on R600, so the problem occurs when a SELECT node has an illegal type,<br>but Operand 0 is a SETCC node with a legal type. I'm not sure the best<br>way to fix this issue. Do you have any suggestions?<br><br>Thanks,<br>Tom<br><br><br><blockquote type="cite">+ } else {<br>+ EVT ETy = Cond.getValueType().getVectorElementType();<br>+ unsigned NumElements = Cond.getValueType().getVectorNumElements();<br>+ EVT VCondTy = EVT::getVectorVT(*DAG.getContext(), ETy, NumElements / 2);<br>+ CL = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, VCondTy, Cond,<br>+ DAG.getConstant(0, TLI.getVectorIdxTy()));<br>+ CH = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, VCondTy, Cond,<br>+ DAG.getConstant(NumElements / 2, TLI.getVectorIdxTy()));<br>+ }<br>}<br><br>Lo = DAG.getNode(N->getOpcode(), dl, LL.getValueType(), CL, LL, RL);<br><br>Modified: llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.h?rev=194542&r1=194541&r2=194542&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.h?rev=194542&r1=194541&r2=194542&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.h (original)<br>+++ llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.h Tue Nov 12 19:57:54 2013<br>@@ -141,8 +141,11 @@ namespace llvm {<br><br>SDValue LowerVASTART(SDValue Op, SelectionDAG &DAG) const;<br>SDValue LowerConstantPool(SDValue Op, SelectionDAG &DAG) const;<br>- virtual EVT getSetCCResultType(LLVMContext &, EVT) const {<br>- return MVT::i1;<br>+ virtual EVT getSetCCResultType(LLVMContext &C, EVT VT) const {<br>+ if (!VT.isVector())<br>+ return MVT::i1;<br>+ else<br>+ return EVT::getVectorVT(C, MVT::i1, VT.getVectorNumElements());<br>}<br><br>virtual bool getPostIndexedAddressParts(SDNode *N, SDNode *Op,<br><br>Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=194542&r1=194541&r2=194542&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=194542&r1=194541&r2=194542&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)<br>+++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Tue Nov 12 19:57:54 2013<br>@@ -1547,7 +1547,16 @@ void X86TargetLowering::resetOperationAc<br>}<br><br>EVT X86TargetLowering::getSetCCResultType(LLVMContext &, EVT VT) const {<br>- if (!VT.isVector()) return MVT::i8;<br>+ if (!VT.isVector())<br>+ return MVT::i8;<br>+<br>+ const TargetMachine &TM = getTargetMachine();<br>+ if (!TM.Options.UseSoftFloat && Subtarget->hasAVX512())<br>+ switch(VT.getVectorNumElements()) {<br>+ case 8: return MVT::v8i1;<br>+ case 16: return MVT::v16i1;<br>+ }<br>+<br>return VT.changeVectorElementTypeToInteger();<br>}<br><br><br>Added: llvm/trunk/test/CodeGen/X86/vec_split.ll<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/vec_split.ll?rev=194542&view=auto">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/vec_split.ll?rev=194542&view=auto</a><br>==============================================================================<br>--- llvm/trunk/test/CodeGen/X86/vec_split.ll (added)<br>+++ llvm/trunk/test/CodeGen/X86/vec_split.ll Tue Nov 12 19:57:54 2013<br>@@ -0,0 +1,42 @@<br>+; RUN: llc -march=x86-64 -mcpu=corei7 < %s | FileCheck %s -check-prefix=SSE4<br>+; RUN: llc -march=x86-64 -mcpu=corei7-avx < %s | FileCheck %s -check-prefix=AVX1<br>+; RUN: llc -march=x86-64 -mcpu=core-avx2 < %s | FileCheck %s -check-prefix=AVX2<br>+<br>+define <16 x i16> @split16(<16 x i16> %a, <16 x i16> %b, <16 x i8> %__mask) {<br>+; SSE4-LABEL: split16:<br>+; SSE4: pminuw<br>+; SSE4: pminuw<br>+; SSE4: ret<br>+; AVX1-LABEL: split16:<br>+; AVX1: vpminuw<br>+; AVX1: vpminuw<br>+; AVX1: ret<br>+; AVX2-LABEL: split16:<br>+; AVX2: vpminuw<br>+; AVX2: ret<br>+ %1 = icmp ult <16 x i16> %a, %b<br>+ %2 = select <16 x i1> %1, <16 x i16> %a, <16 x i16> %b<br>+ ret <16 x i16> %2<br>+}<br>+<br>+define <32 x i16> @split32(<32 x i16> %a, <32 x i16> %b, <32 x i8> %__mask) {<br>+; SSE4-LABEL: split32:<br>+; SSE4: pminuw<br>+; SSE4: pminuw<br>+; SSE4: pminuw<br>+; SSE4: pminuw<br>+; SSE4: ret<br>+; AVX1-LABEL: split32:<br>+; AVX1: vpminuw<br>+; AVX1: vpminuw<br>+; AVX1: vpminuw<br>+; AVX1: vpminuw<br>+; AVX1: ret<br>+; AVX2-LABEL: split32:<br>+; AVX2: vpminuw<br>+; AVX2: vpminuw<br>+; AVX2: ret<br>+ %1 = icmp ult <32 x i16> %a, %b<br>+ %2 = select <32 x i1> %1, <32 x i16> %a, <32 x i16> %b<br>+ ret <32 x i16> %2<br>+}<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></blockquote><0001-R600-Fix-getSetCCResultType-to-handle-new-VSELECT-DA.patch><br></blockquote><br></blockquote><0001-R600-Fix-getSetCCResultType-to-handle-new-VSELECT-DA.patch><0002-SelectionDAG-Fix-assertion-failure-when-splitting-ve.patch><br></blockquote><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></blockquote><br></blockquote></blockquote><br></blockquote><br><span><0001-Split-SETCC-if-VSELECT-requires-splitting-too.patch></span><span><0002-SelectionDAG-Optimize-expansion-of-vec_type-BITCAST-.patch></span></div></blockquote></div><br></div></div>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></blockquote></div><br></div></body></html>