<div dir="ltr">This was bisected to a commit from 2014, so it's not a regression w.r.t 3.8. <div>But it seems like general goodness. <div><br></div><div>Justin?</div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 2, 2016 at 11:21 AM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Should we merge this to 3.9?<br>
<div class="HOEnZb"><div class="h5"><br>
On Mon, Aug 1, 2016 at 12:39 PM, Michael Kuperstein via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
> Author: mkuper<br>
> Date: Mon Aug 1 14:39:49 2016<br>
> New Revision: 277371<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=277371&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=277371&view=rev</a><br>
> Log:<br>
> [DAGCombine] Make sext(setcc) combine respect getBooleanContents<br>
><br>
> We used to combine "sext(setcc x, y, cc) -> (select (setcc x, y, cc), -1, 0)"<br>
> Instead, we should combine to (select (setcc x, y, cc), T, 0) where the value<br>
> of T is 1 or -1, depending on the type of the setcc, and getBooleanContents()<br>
> for the type if it is not i1.<br>
><br>
> This fixes PR28504.<br>
><br>
> Added:<br>
> llvm/trunk/test/CodeGen/X86/pr28504.ll<br>
> Modified:<br>
> llvm/trunk/include/llvm/Target/TargetLowering.h<br>
> llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>
> llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp<br>
><br>
> Modified: llvm/trunk/include/llvm/Target/TargetLowering.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetLowering.h?rev=277371&r1=277370&r2=277371&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetLowering.h?rev=277371&r1=277370&r2=277371&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/Target/TargetLowering.h (original)<br>
> +++ llvm/trunk/include/llvm/Target/TargetLowering.h Mon Aug 1 14:39:49 2016<br>
> @@ -2349,6 +2349,10 @@ public:<br>
> /// from getBooleanContents().<br>
> bool isConstFalseVal(const SDNode *N) const;<br>
><br>
> + /// Return a constant of type VT that contains a true value that respects<br>
> + /// getBooleanContents()<br>
> + SDValue getConstTrueVal(SelectionDAG &DAG, EVT VT, const SDLoc &DL) const;<br>
> +<br>
> /// Return if \p N is a True value when extended to \p VT.<br>
> bool isExtendedTrueVal(const ConstantSDNode *N, EVT VT, bool Signed) const;<br>
><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=277371&r1=277370&r2=277371&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=277371&r1=277370&r2=277371&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Mon Aug 1 14:39:49 2016<br>
> @@ -6198,13 +6198,27 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SD<br>
> }<br>
> }<br>
><br>
> - // sext(setcc x, y, cc) -> (select (setcc x, y, cc), -1, 0)<br>
> - unsigned ElementWidth = VT.getScalarType().getSizeInBits();<br>
> + // sext(setcc x, y, cc) -> (select (setcc x, y, cc), T, 0)<br>
> + // Here, T can be 1 or -1, depending on the type of the setcc and<br>
> + // getBooleanContents().<br>
> + unsigned SetCCWidth = N0.getValueType().getScalarSizeInBits();<br>
> +<br>
> SDLoc DL(N);<br>
> - SDValue NegOne =<br>
> - DAG.getConstant(APInt::getAllOnesValue(ElementWidth), DL, VT);<br>
> + // To determine the "true" side of the select, we need to know the high bit<br>
> + // of the value returned by the setcc if it evaluates to true.<br>
> + // If the type of the setcc is i1, then the true case of the select is just<br>
> + // sext(i1 1), that is, -1.<br>
> + // If the type of the setcc is larger (say, i8) then the value of the high<br>
> + // bit depends on getBooleanContents(). So, ask TLI for a real "true" value<br>
> + // of the appropriate width.<br>
> + SDValue ExtTrueVal =<br>
> + (SetCCWidth == 1)<br>
> + ? DAG.getConstant(APInt::getAllOnesValue(VT.getScalarSizeInBits()),<br>
> + DL, VT)<br>
> + : TLI.getConstTrueVal(DAG, VT, DL);<br>
> +<br>
> if (SDValue SCC = SimplifySelectCC(<br>
> - DL, N0.getOperand(0), N0.getOperand(1), NegOne,<br>
> + DL, N0.getOperand(0), N0.getOperand(1), ExtTrueVal,<br>
> DAG.getConstant(0, DL, VT),<br>
> cast<CondCodeSDNode>(N0.getOperand(2))->get(), true))<br>
> return SCC;<br>
> @@ -6215,10 +6229,10 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SD<br>
> TLI.isOperationLegal(ISD::SETCC, N0.getOperand(0).getValueType())) {<br>
> SDLoc DL(N);<br>
> ISD::CondCode CC = cast<CondCodeSDNode>(N0.getOperand(2))->get();<br>
> - SDValue SetCC = DAG.getSetCC(DL, SetCCVT,<br>
> - N0.getOperand(0), N0.getOperand(1), CC);<br>
> - return DAG.getSelect(DL, VT, SetCC,<br>
> - NegOne, DAG.getConstant(0, DL, VT));<br>
> + SDValue SetCC =<br>
> + DAG.getSetCC(DL, SetCCVT, N0.getOperand(0), N0.getOperand(1), CC);<br>
> + return DAG.getSelect(DL, VT, SetCC, ExtTrueVal,<br>
> + DAG.getConstant(0, DL, VT));<br>
> }<br>
> }<br>
> }<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp?rev=277371&r1=277370&r2=277371&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp?rev=277371&r1=277370&r2=277371&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp Mon Aug 1 14:39:49 2016<br>
> @@ -1234,6 +1234,16 @@ bool TargetLowering::isConstTrueVal(cons<br>
> llvm_unreachable("Invalid boolean contents");<br>
> }<br>
><br>
> +SDValue TargetLowering::getConstTrueVal(SelectionDAG &DAG, EVT VT,<br>
> + const SDLoc &DL) const {<br>
> + unsigned ElementWidth = VT.getScalarSizeInBits();<br>
> + APInt TrueInt =<br>
> + getBooleanContents(VT) == TargetLowering::ZeroOrOneBooleanContent<br>
> + ? APInt(ElementWidth, 1)<br>
> + : APInt::getAllOnesValue(ElementWidth);<br>
> + return DAG.getConstant(TrueInt, DL, VT);<br>
> +}<br>
> +<br>
> bool TargetLowering::isConstFalseVal(const SDNode *N) const {<br>
> if (!N)<br>
> return false;<br>
><br>
> Added: llvm/trunk/test/CodeGen/X86/pr28504.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/pr28504.ll?rev=277371&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/pr28504.ll?rev=277371&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/CodeGen/X86/pr28504.ll (added)<br>
> +++ llvm/trunk/test/CodeGen/X86/pr28504.ll Mon Aug 1 14:39:49 2016<br>
> @@ -0,0 +1,37 @@<br>
> +; RUN: llc -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s<br>
> +<br>
> +; The test case is rather involved, because we need to get to a state where<br>
> +; We have a sext(setcc x, y, cc) -> (select (setcc x, y, cc), T, 0) combine,<br>
> +; BUT this combine is only triggered post-legalization, so the setcc's return<br>
> +; type is i8. So we can't have the combine opportunity be exposed too early.<br>
> +; Basically, what we want to see is that the compare result zero-extended, and<br>
> +; then stored. Only one zext, and no sexts.<br>
> +<br>
> +; CHECK-LABEL: main:<br>
> +; CHECK: movzbl (%rdi), %[[EAX:.*]]<br>
> +; CHECK-NEXT: xorl %e[[C:.]]x, %e[[C]]x<br>
> +; CHECK-NEXT: cmpl $1, %[[EAX]]<br>
> +; CHECK-NEXT: sete %[[C]]l<br>
> +; CHECK-NEXT: movl %e[[C]]x, (%rsi)<br>
> +define void @main(i8* %p, i32* %q) {<br>
> +bb:<br>
> + %tmp4 = load i8, i8* %p, align 1<br>
> + %tmp5 = sext i8 %tmp4 to i32<br>
> + %tmp6 = load i8, i8* %p, align 1<br>
> + %tmp7 = zext i8 %tmp6 to i32<br>
> + %tmp8 = sub nsw i32 %tmp5, %tmp7<br>
> + %tmp11 = icmp eq i32 %tmp7, 1<br>
> + %tmp12 = zext i1 %tmp11 to i32<br>
> + %tmp13 = add nsw i32 %tmp8, %tmp12<br>
> + %tmp14 = trunc i32 %tmp13 to i8<br>
> + %tmp15 = sext i8 %tmp14 to i16<br>
> + %tmp16 = sext i16 %tmp15 to i32<br>
> + store i32 %tmp16, i32* %q, align 4<br>
> + br i1 %tmp11, label %bb21, label %bb22<br>
> +<br>
> +bb21: ; preds = %bb<br>
> + unreachable<br>
> +<br>
> +bb22: ; preds = %bb<br>
> + ret void<br>
> +}<br>
><br>
><br>
> _______________________________________________<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/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>