From 11596ed43c5eb3b8a49f563abdbdb7ea54fec991 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 9 Oct 2009 20:35:19 +0000 Subject: [PATCH] Fix the x86 test-shrink optimization so that it doesn't shrink comparisons when one of the bits being tested would end up being the sign bit in the narrower type, and a signed comparison is being performed, since this would change the result of the signed comparison. This fixes PR5132. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@83670 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/X86/X86ISelDAGToDAG.cpp | 78 +++++++++++++++++++++++++++-- test/CodeGen/X86/test-shrink-bug.ll | 23 +++++++++ 2 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 test/CodeGen/X86/test-shrink-bug.ll diff --git a/lib/Target/X86/X86ISelDAGToDAG.cpp b/lib/Target/X86/X86ISelDAGToDAG.cpp index 71b4062816f..5b678fb602d 100644 --- a/lib/Target/X86/X86ISelDAGToDAG.cpp +++ b/lib/Target/X86/X86ISelDAGToDAG.cpp @@ -1625,6 +1625,68 @@ SDNode *X86DAGToDAGISel::SelectAtomicLoadAdd(SDNode *Node, EVT NVT) { } } +/// HasNoSignedComparisonUses - Test whether the given X86ISD::CMP node has +/// any uses which require the SF or OF bits to be accurate. +static bool HasNoSignedComparisonUses(SDNode *N) { + // Examine each user of the node. + for (SDNode::use_iterator UI = N->use_begin(), + UE = N->use_end(); UI != UE; ++UI) { + // Only examine CopyToReg uses. + if (UI->getOpcode() != ISD::CopyToReg) + return false; + // Only examine CopyToReg uses that copy to EFLAGS. + if (cast(UI->getOperand(1))->getReg() != + X86::EFLAGS) + return false; + // Examine each user of the CopyToReg use. + for (SDNode::use_iterator FlagUI = UI->use_begin(), + FlagUE = UI->use_end(); FlagUI != FlagUE; ++FlagUI) { + // Only examine the Flag result. + if (FlagUI.getUse().getResNo() != 1) continue; + // Anything unusual: assume conservatively. + if (!FlagUI->isMachineOpcode()) return false; + // Examine the opcode of the user. + switch (FlagUI->getMachineOpcode()) { + // These comparisons don't treat the most significant bit specially. + case X86::SETAr: case X86::SETAEr: case X86::SETBr: case X86::SETBEr: + case X86::SETEr: case X86::SETNEr: case X86::SETPr: case X86::SETNPr: + case X86::SETAm: case X86::SETAEm: case X86::SETBm: case X86::SETBEm: + case X86::SETEm: case X86::SETNEm: case X86::SETPm: case X86::SETNPm: + case X86::JA: case X86::JAE: case X86::JB: case X86::JBE: + case X86::JE: case X86::JNE: case X86::JP: case X86::JNP: + case X86::CMOVA16rr: case X86::CMOVA16rm: + case X86::CMOVA32rr: case X86::CMOVA32rm: + case X86::CMOVA64rr: case X86::CMOVA64rm: + case X86::CMOVAE16rr: case X86::CMOVAE16rm: + case X86::CMOVAE32rr: case X86::CMOVAE32rm: + case X86::CMOVAE64rr: case X86::CMOVAE64rm: + case X86::CMOVB16rr: case X86::CMOVB16rm: + case X86::CMOVB32rr: case X86::CMOVB32rm: + case X86::CMOVB64rr: case X86::CMOVB64rm: + case X86::CMOVBE16rr: case X86::CMOVBE16rm: + case X86::CMOVBE32rr: case X86::CMOVBE32rm: + case X86::CMOVBE64rr: case X86::CMOVBE64rm: + case X86::CMOVE16rr: case X86::CMOVE16rm: + case X86::CMOVE32rr: case X86::CMOVE32rm: + case X86::CMOVE64rr: case X86::CMOVE64rm: + case X86::CMOVNE16rr: case X86::CMOVNE16rm: + case X86::CMOVNE32rr: case X86::CMOVNE32rm: + case X86::CMOVNE64rr: case X86::CMOVNE64rm: + case X86::CMOVNP16rr: case X86::CMOVNP16rm: + case X86::CMOVNP32rr: case X86::CMOVNP32rm: + case X86::CMOVNP64rr: case X86::CMOVNP64rm: + case X86::CMOVP16rr: case X86::CMOVP16rm: + case X86::CMOVP32rr: case X86::CMOVP32rm: + case X86::CMOVP64rr: case X86::CMOVP64rm: + continue; + // Anything else: assume conservatively. + default: return false; + } + } + } + return true; +} + SDNode *X86DAGToDAGISel::Select(SDValue N) { SDNode *Node = N.getNode(); EVT NVT = Node->getValueType(0); @@ -1978,7 +2040,9 @@ SDNode *X86DAGToDAGISel::Select(SDValue N) { if (!C) break; // For example, convert "testl %eax, $8" to "testb %al, $8" - if ((C->getZExtValue() & ~UINT64_C(0xff)) == 0) { + if ((C->getZExtValue() & ~UINT64_C(0xff)) == 0 && + (!(C->getZExtValue() & 0x80) || + HasNoSignedComparisonUses(Node))) { SDValue Imm = CurDAG->getTargetConstant(C->getZExtValue(), MVT::i8); SDValue Reg = N0.getNode()->getOperand(0); @@ -2004,7 +2068,9 @@ SDNode *X86DAGToDAGISel::Select(SDValue N) { } // For example, "testl %eax, $2048" to "testb %ah, $8". - if ((C->getZExtValue() & ~UINT64_C(0xff00)) == 0) { + if ((C->getZExtValue() & ~UINT64_C(0xff00)) == 0 && + (!(C->getZExtValue() & 0x8000) || + HasNoSignedComparisonUses(Node))) { // Shift the immediate right by 8 bits. SDValue ShiftedImm = CurDAG->getTargetConstant(C->getZExtValue() >> 8, MVT::i8); @@ -2034,7 +2100,9 @@ SDNode *X86DAGToDAGISel::Select(SDValue N) { // For example, "testl %eax, $32776" to "testw %ax, $32776". if ((C->getZExtValue() & ~UINT64_C(0xffff)) == 0 && - N0.getValueType() != MVT::i16) { + N0.getValueType() != MVT::i16 && + (!(C->getZExtValue() & 0x8000) || + HasNoSignedComparisonUses(Node))) { SDValue Imm = CurDAG->getTargetConstant(C->getZExtValue(), MVT::i16); SDValue Reg = N0.getNode()->getOperand(0); @@ -2048,7 +2116,9 @@ SDNode *X86DAGToDAGISel::Select(SDValue N) { // For example, "testq %rax, $268468232" to "testl %eax, $268468232". if ((C->getZExtValue() & ~UINT64_C(0xffffffff)) == 0 && - N0.getValueType() == MVT::i64) { + N0.getValueType() == MVT::i64 && + (!(C->getZExtValue() & 0x80000000) || + HasNoSignedComparisonUses(Node))) { SDValue Imm = CurDAG->getTargetConstant(C->getZExtValue(), MVT::i32); SDValue Reg = N0.getNode()->getOperand(0); diff --git a/test/CodeGen/X86/test-shrink-bug.ll b/test/CodeGen/X86/test-shrink-bug.ll new file mode 100644 index 00000000000..64631ea5fc9 --- /dev/null +++ b/test/CodeGen/X86/test-shrink-bug.ll @@ -0,0 +1,23 @@ +; RUN: llc < %s | FileCheck %s + +; Codegen shouldn't reduce the comparison down to testb $-1, %al +; because that changes the result of the signed test. +; PR5132 +; CHECK: testw $255, %ax + +target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128" +target triple = "i386-apple-darwin10.0" + +@g_14 = global i8 -6, align 1 ; [#uses=1] + +declare i32 @func_16(i8 signext %p_19, i32 %p_20) nounwind + +define i32 @func_35(i64 %p_38) nounwind ssp { +entry: + %tmp = load i8* @g_14 ; [#uses=2] + %conv = zext i8 %tmp to i32 ; [#uses=1] + %cmp = icmp sle i32 1, %conv ; [#uses=1] + %conv2 = zext i1 %cmp to i32 ; [#uses=1] + %call = call i32 @func_16(i8 signext %tmp, i32 %conv2) ssp ; [#uses=1] + ret i32 1 +} -- 2.34.1