[llvm] [MSP430] Compute c+1 at the operand bit width in EmitCMP (PR #195892)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-msp430

Author: Byeongjee Kang (byeongjee)

<details>
<summary>Changes</summary>

 EmitCMP folds `c CMP rhs` into `rhs CMP' c+1` for four condition codes.
  The `c+1` must wrap modulo the i16 operand width, but the current code
  does `C->getSExtValue() + 1` — sign-extending to int64_t and adding there
  loses the bit width. For constants with bit 15 set the result carries
  bits above bit 15 and violates the contract of
  `getConstant(uint64_t, ..., MVT::i16)`: the `isUIntN` assertion fires
  under `LLVM_ENABLE_ASSERTIONS`, and the resulting APInt has non-zero
  bits past its declared width otherwise.

  Switching to `C->getAPIntValue() + 1` keeps the arithmetic at the
  constant's natural width with modular overflow. Mirrors the recent
  AMDGPU fix in 4f0b96f643d3.

  ## Reproducer (assertions build)
  ```c++
  echo 'define i16 @<!-- -->f(i16 %a) { %t = icmp ugt i16 %a, 32768
    %r = zext i1 %t to i16
    ret i16 %r }' | llc -mtriple=msp430
  # Assertion failed: (llvm::isUIntN(BitWidth, val) ...), APInt.h
  ```
  
  This is my first PR in LLVM. Please let me know if anything needs to be fixed!

---
Full diff: https://github.com/llvm/llvm-project/pull/195892.diff


2 Files Affected:

- (modified) llvm/lib/Target/MSP430/MSP430ISelLowering.cpp (+4-4) 
- (added) llvm/test/CodeGen/MSP430/cmp-imm-high-bit.ll (+71) 


``````````diff
diff --git a/llvm/lib/Target/MSP430/MSP430ISelLowering.cpp b/llvm/lib/Target/MSP430/MSP430ISelLowering.cpp
index e118169377fe2..218c79b283c27 100644
--- a/llvm/lib/Target/MSP430/MSP430ISelLowering.cpp
+++ b/llvm/lib/Target/MSP430/MSP430ISelLowering.cpp
@@ -889,7 +889,7 @@ static SDValue EmitCMP(SDValue &LHS, SDValue &RHS, SDValue &TargetCC,
     // fold constant into instruction.
     if (const ConstantSDNode * C = dyn_cast<ConstantSDNode>(LHS)) {
       LHS = RHS;
-      RHS = DAG.getConstant(C->getSExtValue() + 1, dl, C->getValueType(0));
+      RHS = DAG.getConstant(C->getAPIntValue() + 1, dl, C->getValueType(0));
       TCC = MSP430CC::COND_LO;
       break;
     }
@@ -903,7 +903,7 @@ static SDValue EmitCMP(SDValue &LHS, SDValue &RHS, SDValue &TargetCC,
     // fold constant into instruction.
     if (const ConstantSDNode * C = dyn_cast<ConstantSDNode>(LHS)) {
       LHS = RHS;
-      RHS = DAG.getConstant(C->getSExtValue() + 1, dl, C->getValueType(0));
+      RHS = DAG.getConstant(C->getAPIntValue() + 1, dl, C->getValueType(0));
       TCC = MSP430CC::COND_HS;
       break;
     }
@@ -917,7 +917,7 @@ static SDValue EmitCMP(SDValue &LHS, SDValue &RHS, SDValue &TargetCC,
     // fold constant into instruction.
     if (const ConstantSDNode * C = dyn_cast<ConstantSDNode>(LHS)) {
       LHS = RHS;
-      RHS = DAG.getConstant(C->getSExtValue() + 1, dl, C->getValueType(0));
+      RHS = DAG.getConstant(C->getAPIntValue() + 1, dl, C->getValueType(0));
       TCC = MSP430CC::COND_L;
       break;
     }
@@ -931,7 +931,7 @@ static SDValue EmitCMP(SDValue &LHS, SDValue &RHS, SDValue &TargetCC,
     // fold constant into instruction.
     if (const ConstantSDNode * C = dyn_cast<ConstantSDNode>(LHS)) {
       LHS = RHS;
-      RHS = DAG.getConstant(C->getSExtValue() + 1, dl, C->getValueType(0));
+      RHS = DAG.getConstant(C->getAPIntValue() + 1, dl, C->getValueType(0));
       TCC = MSP430CC::COND_GE;
       break;
     }
diff --git a/llvm/test/CodeGen/MSP430/cmp-imm-high-bit.ll b/llvm/test/CodeGen/MSP430/cmp-imm-high-bit.ll
new file mode 100644
index 0000000000000..5f3f3ecf20700
--- /dev/null
+++ b/llvm/test/CodeGen/MSP430/cmp-imm-high-bit.ll
@@ -0,0 +1,71 @@
+; RUN: llc -mtriple=msp430 < %s | FileCheck %s
+
+; Regression tests for EmitCMP folding of LHS constants whose i16 high bit is
+; set. These exercise the four EmitCMP branches that turn `c CMP rhs` into
+; `rhs CMP' c+1`. The constant addition must use APInt arithmetic at the
+; original bit width; computing it via getSExtValue() and then handing the
+; resulting int64_t to getConstant(uint64_t, ..., MVT::i16) trips the
+; isUIntN(16, val) assertion in the APInt constructor for any constant with
+; bit 15 set.
+
+target datalayout = "e-p:16:16:16-i1:8:8-i8:8:8-i16:16:16-i32:16:32"
+target triple = "msp430-generic-generic"
+
+; CHECK-LABEL: cmp_ule_high_bit:
+; Folded into `rhs u< 0x8001` (c = 0x8000, c+1 = 0x8001 = -32767 signed).
+; CHECK: cmp #-32767, r12
+define i16 @cmp_ule_high_bit(i16 %a) nounwind {
+  %t = icmp ule i16 %a, 32768
+  %r = zext i1 %t to i16
+  ret i16 %r
+}
+
+; CHECK-LABEL: cmp_ugt_high_bit:
+; Folded into `rhs u>= 0x8001`.
+; CHECK: cmp #-32767, r12
+define i16 @cmp_ugt_high_bit(i16 %a) nounwind {
+  %t = icmp ugt i16 %a, 32768
+  %r = zext i1 %t to i16
+  ret i16 %r
+}
+
+; CHECK-LABEL: cmp_sle_neg_high_bit:
+; Folded into `rhs s< -32766` (c = -32767, c+1 = -32766).
+; CHECK: cmp #-32766,
+define i16 @cmp_sle_neg_high_bit(i16 %a) nounwind {
+  %t = icmp sle i16 %a, -32767
+  %r = zext i1 %t to i16
+  ret i16 %r
+}
+
+; CHECK-LABEL: cmp_sgt_neg_high_bit:
+; Folded into `rhs s>= -32766`.
+; CHECK: cmp #-32766,
+define i16 @cmp_sgt_neg_high_bit(i16 %a) nounwind {
+  %t = icmp sgt i16 %a, -32767
+  %r = zext i1 %t to i16
+  ret i16 %r
+}
+
+; The br_cc path goes through the same EmitCMP helper.
+; CHECK-LABEL: br_ule_high_bit:
+; CHECK: cmp #-32767, r12
+define i16 @br_ule_high_bit(i16 %a) nounwind {
+  %t = icmp ule i16 %a, 32768
+  br i1 %t, label %yes, label %no
+yes:
+  ret i16 1
+no:
+  ret i16 0
+}
+
+; CHECK-LABEL: br_sle_neg_high_bit:
+; CHECK: cmp #-32766, r12
+define i16 @br_sle_neg_high_bit(i16 %a) nounwind {
+  %t = icmp sle i16 %a, -32767
+  br i1 %t, label %yes, label %no
+yes:
+  ret i16 1
+no:
+  ret i16 0
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/195892