From c97da7f3a44c2358cc50737e1567cd69d2abe695 Mon Sep 17 00:00:00 2001 From: Michael Kuperstein Date: Mon, 1 Aug 2016 19:39:49 +0000 Subject: [PATCH] [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. llvm-svn: 277371 --- llvm/include/llvm/Target/TargetLowering.h | 4 +++ llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 32 ++++++++++++++------ llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp | 10 +++++++ llvm/test/CodeGen/X86/pr28504.ll | 37 ++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 9 deletions(-) create mode 100644 llvm/test/CodeGen/X86/pr28504.ll diff --git a/llvm/include/llvm/Target/TargetLowering.h b/llvm/include/llvm/Target/TargetLowering.h index d21d321..4586a17 100644 --- a/llvm/include/llvm/Target/TargetLowering.h +++ b/llvm/include/llvm/Target/TargetLowering.h @@ -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; diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index b8afe41..6a74430 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -6198,13 +6198,27 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SDNode *N) { } } - // 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(N0.getOperand(2))->get(), true)) return SCC; @@ -6215,10 +6229,10 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SDNode *N) { TLI.isOperationLegal(ISD::SETCC, N0.getOperand(0).getValueType())) { SDLoc DL(N); ISD::CondCode CC = cast(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)); } } } diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp index 3679810..bb67aa3 100644 --- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp @@ -1234,6 +1234,16 @@ bool TargetLowering::isConstTrueVal(const SDNode *N) const { 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; diff --git a/llvm/test/CodeGen/X86/pr28504.ll b/llvm/test/CodeGen/X86/pr28504.ll new file mode 100644 index 0000000..a617c8a --- /dev/null +++ b/llvm/test/CodeGen/X86/pr28504.ll @@ -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 +} -- 2.7.4