[llvm] r297820 - [Thumb1] Fix the bug when adding/subtracting -2147483648

[Friedman, Eli via llvm-commits]

On 3/15/2017 3:19 AM, Artyom Skrobov via llvm-commits wrote:
> Author: askrobov
> Date: Wed Mar 15 05:19:16 2017
> New Revision: 297820
>
> URL: http://llvm.org/viewvc/llvm-project?rev=297820&view=rev
> Log:
> [Thumb1] Fix the bug when adding/subtracting -2147483648
>
> Differential Revision: https://reviews.llvm.org/D30829
>
> Modified:
>      llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
>      llvm/trunk/test/CodeGen/Thumb/long.ll
>
> Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=297820&r1=297819&r2=297820&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Wed Mar 15 05:19:16 2017
> @@ -9788,8 +9788,8 @@ static SDValue PerformAddcSubcCombine(SD
>     if (Subtarget->isThumb1Only()) {
>       SDValue RHS = N->getOperand(1);
>       if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(RHS)) {
> -      int64_t imm = C->getSExtValue();
> -      if (imm < 0) {
> +      int32_t imm = C->getSExtValue();
> +      if (-imm > 0) {

This has undefined behavior if imm==INT32_MIN.  Probably better to just 
explicitly check for it.

>           SDLoc DL(N);
>           RHS = DAG.getConstant(-imm, DL, MVT::i32);
>           unsigned Opcode = (N->getOpcode() == ARMISD::ADDC) ? ARMISD::SUBC
> @@ -9806,8 +9806,8 @@ static SDValue PerformAddeSubeCombine(SD
>     if (Subtarget->isThumb1Only()) {
>       SDValue RHS = N->getOperand(1);
>       if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(RHS)) {
> -      int64_t imm = C->getSExtValue();
> -      if (imm < 0) {
> +      int32_t imm = C->getSExtValue();
> +      if (-imm > 0) {
>           SDLoc DL(N);
>   
>           // The with-carry-in form matches bitwise not instead of the negation.

The change to PerformAddeSubeCombine isn't necessary: it uses a logical 
not, which always clears the sign bit.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project