From adea1de7ba14906134092ce1c30bc372a737a654 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Fri, 27 Jul 2018 05:56:27 +0000 Subject: [PATCH] [X86] Remove an unnecessary 'if' that prevented treating INT64_MAX and -INT64_MAX as power of 2 minus 1 in the multiply expansion code. Not sure why they were being explicitly excluded, but I believe all the math inside the if works. I changed the absolute value to be uint64_t instead of int64_t so INT64_MIN+1 wouldn't be signed wrap. llvm-svn: 338101 --- lib/Target/X86/X86ISelLowering.cpp | 74 +++++++++++++++--------------- test/CodeGen/X86/imul.ll | 29 +++++++++++- 2 files changed, 63 insertions(+), 40 deletions(-) diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp index e827c5c35f1..7dcdb796705 100644 --- a/lib/Target/X86/X86ISelLowering.cpp +++ b/lib/Target/X86/X86ISelLowering.cpp @@ -33928,45 +33928,43 @@ static SDValue combineMul(SDNode *N, SelectionDAG &DAG, "Both cases that could cause potential overflows should have " "already been handled."); int64_t SignMulAmt = C->getSExtValue(); - if ((SignMulAmt != INT64_MIN) && (SignMulAmt != INT64_MAX) && - (SignMulAmt != -INT64_MAX)) { - int64_t AbsMulAmt = SignMulAmt < 0 ? -SignMulAmt : SignMulAmt; - if (isPowerOf2_64(AbsMulAmt - 1)) { - // (mul x, 2^N + 1) => (add (shl x, N), x) - NewMul = DAG.getNode( - ISD::ADD, DL, VT, N->getOperand(0), - DAG.getNode(ISD::SHL, DL, VT, N->getOperand(0), - DAG.getConstant(Log2_64(AbsMulAmt - 1), DL, - MVT::i8))); - // To negate, subtract the number from zero - if (SignMulAmt < 0) - NewMul = DAG.getNode(ISD::SUB, DL, VT, - DAG.getConstant(0, DL, VT), NewMul); - } else if (isPowerOf2_64(AbsMulAmt + 1)) { - // (mul x, 2^N - 1) => (sub (shl x, N), x) - NewMul = DAG.getNode(ISD::SHL, DL, VT, N->getOperand(0), - DAG.getConstant(Log2_64(AbsMulAmt + 1), - DL, MVT::i8)); - // To negate, reverse the operands of the subtract. - if (SignMulAmt < 0) - NewMul = DAG.getNode(ISD::SUB, DL, VT, N->getOperand(0), NewMul); - else - NewMul = DAG.getNode(ISD::SUB, DL, VT, NewMul, N->getOperand(0)); - } else if (SignMulAmt >= 0 && isPowerOf2_64(AbsMulAmt - 2)) { - // (mul x, 2^N + 2) => (add (add (shl x, N), x), x) - NewMul = DAG.getNode(ISD::SHL, DL, VT, N->getOperand(0), - DAG.getConstant(Log2_64(AbsMulAmt - 2), - DL, MVT::i8)); - NewMul = DAG.getNode(ISD::ADD, DL, VT, NewMul, N->getOperand(0)); - NewMul = DAG.getNode(ISD::ADD, DL, VT, NewMul, N->getOperand(0)); - } else if (SignMulAmt >= 0 && isPowerOf2_64(AbsMulAmt + 2)) { - // (mul x, 2^N - 2) => (sub (sub (shl x, N), x), x) - NewMul = DAG.getNode(ISD::SHL, DL, VT, N->getOperand(0), - DAG.getConstant(Log2_64(AbsMulAmt + 2), - DL, MVT::i8)); + assert(SignMulAmt != INT64_MIN && "Int min should have been handled!"); + uint64_t AbsMulAmt = SignMulAmt < 0 ? -SignMulAmt : SignMulAmt; + if (isPowerOf2_64(AbsMulAmt - 1)) { + // (mul x, 2^N + 1) => (add (shl x, N), x) + NewMul = DAG.getNode( + ISD::ADD, DL, VT, N->getOperand(0), + DAG.getNode(ISD::SHL, DL, VT, N->getOperand(0), + DAG.getConstant(Log2_64(AbsMulAmt - 1), DL, + MVT::i8))); + // To negate, subtract the number from zero + if (SignMulAmt < 0) + NewMul = DAG.getNode(ISD::SUB, DL, VT, + DAG.getConstant(0, DL, VT), NewMul); + } else if (isPowerOf2_64(AbsMulAmt + 1)) { + // (mul x, 2^N - 1) => (sub (shl x, N), x) + NewMul = DAG.getNode(ISD::SHL, DL, VT, N->getOperand(0), + DAG.getConstant(Log2_64(AbsMulAmt + 1), + DL, MVT::i8)); + // To negate, reverse the operands of the subtract. + if (SignMulAmt < 0) + NewMul = DAG.getNode(ISD::SUB, DL, VT, N->getOperand(0), NewMul); + else NewMul = DAG.getNode(ISD::SUB, DL, VT, NewMul, N->getOperand(0)); - NewMul = DAG.getNode(ISD::SUB, DL, VT, NewMul, N->getOperand(0)); - } + } else if (SignMulAmt >= 0 && isPowerOf2_64(AbsMulAmt - 2)) { + // (mul x, 2^N + 2) => (add (add (shl x, N), x), x) + NewMul = DAG.getNode(ISD::SHL, DL, VT, N->getOperand(0), + DAG.getConstant(Log2_64(AbsMulAmt - 2), + DL, MVT::i8)); + NewMul = DAG.getNode(ISD::ADD, DL, VT, NewMul, N->getOperand(0)); + NewMul = DAG.getNode(ISD::ADD, DL, VT, NewMul, N->getOperand(0)); + } else if (SignMulAmt >= 0 && isPowerOf2_64(AbsMulAmt + 2)) { + // (mul x, 2^N - 2) => (sub (sub (shl x, N), x), x) + NewMul = DAG.getNode(ISD::SHL, DL, VT, N->getOperand(0), + DAG.getConstant(Log2_64(AbsMulAmt + 2), + DL, MVT::i8)); + NewMul = DAG.getNode(ISD::SUB, DL, VT, NewMul, N->getOperand(0)); + NewMul = DAG.getNode(ISD::SUB, DL, VT, NewMul, N->getOperand(0)); } } diff --git a/test/CodeGen/X86/imul.ll b/test/CodeGen/X86/imul.ll index 14f15143b95..0f1a824f00f 100644 --- a/test/CodeGen/X86/imul.ll +++ b/test/CodeGen/X86/imul.ll @@ -499,8 +499,9 @@ entry: define i64 @testOverflow(i64 %a) { ; X64-LABEL: testOverflow: ; X64: # %bb.0: # %entry -; X64-NEXT: movabsq $9223372036854775807, %rax # imm = 0x7FFFFFFFFFFFFFFF -; X64-NEXT: imulq %rdi, %rax +; X64-NEXT: movq %rdi, %rax +; X64-NEXT: shlq $63, %rax +; X64-NEXT: subq %rdi, %rax ; X64-NEXT: retq ; ; X86-LABEL: testOverflow: @@ -524,3 +525,27 @@ entry: %tmp3 = mul i64 %a, 9223372036854775807 ret i64 %tmp3 } + +define i64 @testNegOverflow(i64 %a) { +; X64-LABEL: testNegOverflow: +; X64: # %bb.0: # %entry +; X64-NEXT: movq %rdi, %rax +; X64-NEXT: shlq $63, %rax +; X64-NEXT: subq %rax, %rdi +; X64-NEXT: movq %rdi, %rax +; X64-NEXT: retq +; +; X86-LABEL: testNegOverflow: +; X86: # %bb.0: # %entry +; X86-NEXT: movl {{[0-9]+}}(%esp), %ecx +; X86-NEXT: movl $1, %edx +; X86-NEXT: movl %ecx, %eax +; X86-NEXT: mull %edx +; X86-NEXT: shll $31, %ecx +; X86-NEXT: addl %ecx, %edx +; X86-NEXT: addl {{[0-9]+}}(%esp), %edx +; X86-NEXT: retl +entry: + %tmp3 = mul i64 %a, -9223372036854775807 + ret i64 %tmp3 +}