[llvm] r277371 - [DAGCombine] Make sext(setcc) combine respect getBooleanContents
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 2 11:48:12 PDT 2016
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.
> 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