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