<div dir="ltr">Test case added in r312335</div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature" data-smartmail="gmail_signature">~Craig</div></div>
<br><div class="gmail_quote">On Fri, Sep 1, 2017 at 9:37 AM, Topper, Craig via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Probably forgot to 'git add' it. Sorry. I'll do it now.<br>
<div class="HOEnZb"><div class="h5"><br>
-----Original Message-----<br>
From: <a href="mailto:hwennborg@google.com">hwennborg@google.com</a> [mailto:<a href="mailto:hwennborg@google.com">hwennborg@google.com</a>] On Behalf Of Hans Wennborg<br>
Sent: Friday, September 01, 2017 9:31 AM<br>
To: Topper, Craig <<a href="mailto:craig.topper@intel.com">craig.topper@intel.com</a>><br>
Cc: llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>><br>
Subject: Re: [llvm] r312285 - [X86] Don't pull carry through X86ISD::ADD carryin, -1 if we can't guranteed we're really using the carry flag from the add.<br>
<br>
Merged to 5.0 in r312333.<br>
<br>
Did you forget to commit the test case?<br>
<br>
On Thu, Aug 31, 2017 at 2:39 PM, Craig Topper via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
> Author: ctopper<br>
> Date: Thu Aug 31 14:39:23 2017<br>
> New Revision: 312285<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=312285&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=312285&view=rev</a><br>
> Log:<br>
> [X86] Don't pull carry through X86ISD::ADD carryin, -1 if we can't guranteed we're really using the carry flag from the add.<br>
><br>
> Prior to this patch we had a DAG combine that tried to bypass an X86ISD::ADD with -1 being added to the carry flag of some previous operation. We would then pass the carry flag directly to user.<br>
><br>
> But this is only safe if the user is looking for the carry flag and not the zero flag.<br>
><br>
> So we need to only do this combine in a context where we know what flag the consumer is using.<br>
><br>
> Fixes PR34381.<br>
><br>
> Differential Revision: <a href="https://reviews.llvm.org/D37317" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D37317</a><br>
><br>
> Modified:<br>
> llvm/trunk/lib/Target/X86/<wbr>X86ISelLowering.cpp<br>
><br>
> Modified: llvm/trunk/lib/Target/X86/<wbr>X86ISelLowering.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=312285&r1=312284&r2=312285&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/Target/<wbr>X86/X86ISelLowering.cpp?rev=<wbr>312285&r1=312284&r2=312285&<wbr>view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/lib/Target/X86/<wbr>X86ISelLowering.cpp (original)<br>
> +++ llvm/trunk/lib/Target/X86/<wbr>X86ISelLowering.cpp Thu Aug 31 14:39:23 2017<br>
> @@ -30939,11 +30939,40 @@ static bool checkBoolTestAndOrSetCCCombi<br>
> return true;<br>
> }<br>
><br>
> +// When legalizing carry, we create carries via add X, -1<br>
> +// If that comes from an actual carry, via setcc, we use the<br>
> +// carry directly.<br>
> +static SDValue combineCarryThroughADD(SDValue EFLAGS) {<br>
> + if (EFLAGS.getOpcode() == X86ISD::ADD) {<br>
> + if (isAllOnesConstant(EFLAGS.<wbr>getOperand(1))) {<br>
> + SDValue Carry = EFLAGS.getOperand(0);<br>
> + while (Carry.getOpcode() == ISD::TRUNCATE ||<br>
> + Carry.getOpcode() == ISD::ZERO_EXTEND ||<br>
> + Carry.getOpcode() == ISD::SIGN_EXTEND ||<br>
> + Carry.getOpcode() == ISD::ANY_EXTEND ||<br>
> + (Carry.getOpcode() == ISD::AND &&<br>
> + isOneConstant(Carry.<wbr>getOperand(1))))<br>
> + Carry = Carry.getOperand(0);<br>
> + if (Carry.getOpcode() == X86ISD::SETCC ||<br>
> + Carry.getOpcode() == X86ISD::SETCC_CARRY) {<br>
> + if (Carry.getConstantOperandVal(<wbr>0) == X86::COND_B)<br>
> + return Carry.getOperand(1);<br>
> + }<br>
> + }<br>
> + }<br>
> +<br>
> + return SDValue();<br>
> +}<br>
> +<br>
> /// Optimize an EFLAGS definition used according to the condition code \p CC<br>
> /// into a simpler EFLAGS value, potentially returning a new \p CC and replacing<br>
> /// uses of chain values.<br>
> static SDValue combineSetCCEFLAGS(SDValue EFLAGS, X86::CondCode &CC,<br>
> SelectionDAG &DAG) {<br>
> + if (CC == X86::COND_B)<br>
> + if (SDValue Flags = combineCarryThroughADD(EFLAGS)<wbr>)<br>
> + return Flags;<br>
> +<br>
> if (SDValue R = checkBoolTestSetCCCombine(<wbr>EFLAGS, CC))<br>
> return R;<br>
> return combineSetCCAtomicArith(<wbr>EFLAGS, CC, DAG);<br>
> @@ -34987,27 +35016,13 @@ static SDValue combineSIntToFP(SDNode *N<br>
> return SDValue();<br>
> }<br>
><br>
> -// Optimize RES, EFLAGS = X86ISD::ADD LHS, RHS<br>
> -static SDValue combineX86ADD(SDNode *N, SelectionDAG &DAG,<br>
> - X86TargetLowering::<wbr>DAGCombinerInfo &DCI) {<br>
> - // When legalizing carry, we create carries via add X, -1<br>
> - // If that comes from an actual carry, via setcc, we use the<br>
> - // carry directly.<br>
> - if (isAllOnesConstant(N-><wbr>getOperand(1)) && N->hasAnyUseOfValue(1)) {<br>
> - SDValue Carry = N->getOperand(0);<br>
> - while (Carry.getOpcode() == ISD::TRUNCATE ||<br>
> - Carry.getOpcode() == ISD::ZERO_EXTEND ||<br>
> - Carry.getOpcode() == ISD::SIGN_EXTEND ||<br>
> - Carry.getOpcode() == ISD::ANY_EXTEND ||<br>
> - (Carry.getOpcode() == ISD::AND &&<br>
> - isOneConstant(Carry.<wbr>getOperand(1))))<br>
> - Carry = Carry.getOperand(0);<br>
> -<br>
> - if (Carry.getOpcode() == X86ISD::SETCC ||<br>
> - Carry.getOpcode() == X86ISD::SETCC_CARRY) {<br>
> - if (Carry.getConstantOperandVal(<wbr>0) == X86::COND_B)<br>
> - return DCI.CombineTo(N, SDValue(N, 0), Carry.getOperand(1));<br>
> - }<br>
> +static SDValue combineSBB(SDNode *N, SelectionDAG &DAG) {<br>
> + if (SDValue Flags = combineCarryThroughADD(N-><wbr>getOperand(2))) {<br>
> + MVT VT = N->getSimpleValueType(0);<br>
> + SDVTList VTs = DAG.getVTList(VT, MVT::i32);<br>
> + return DAG.getNode(X86ISD::SBB, SDLoc(N), VTs,<br>
> + N->getOperand(0), N->getOperand(1),<br>
> + Flags);<br>
> }<br>
><br>
> return SDValue();<br>
> @@ -35036,6 +35051,14 @@ static SDValue combineADC(SDNode *N, Sel<br>
> return DCI.CombineTo(N, Res1, CarryOut);<br>
> }<br>
><br>
> + if (SDValue Flags = combineCarryThroughADD(N-><wbr>getOperand(2))) {<br>
> + MVT VT = N->getSimpleValueType(0);<br>
> + SDVTList VTs = DAG.getVTList(VT, MVT::i32);<br>
> + return DAG.getNode(X86ISD::ADC, SDLoc(N), VTs,<br>
> + N->getOperand(0), N->getOperand(1),<br>
> + Flags);<br>
> + }<br>
> +<br>
> return SDValue();<br>
> }<br>
><br>
> @@ -35721,7 +35744,7 @@ SDValue X86TargetLowering::<wbr>PerformDAGCom<br>
> case X86ISD::CMOV: return combineCMov(N, DAG, DCI, Subtarget);<br>
> case ISD::ADD: return combineAdd(N, DAG, Subtarget);<br>
> case ISD::SUB: return combineSub(N, DAG, Subtarget);<br>
> - case X86ISD::ADD: return combineX86ADD(N, DAG, DCI);<br>
> + case X86ISD::SBB: return combineSBB(N, DAG);<br>
> case X86ISD::ADC: return combineADC(N, DAG, DCI);<br>
> case ISD::MUL: return combineMul(N, DAG, DCI, Subtarget);<br>
> case ISD::SHL:<br>
><br>
><br>
> ______________________________<wbr>_________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>