[ARM] t2_so_imm_neg had a subtle bug in the conversion, and could trigger UB by negating (int)-2147483648. By pure luck, none of the pre-existing tests triggered this; so I'm adding one.

[tyomitch]

Summary

Thanks to Vitaly Buka for helping catch this.

[rengolin]

lib/Target/ARM/ARMInstrThumb2.td

115

Why not just use INT_MIN?

[tyomitch]

lib/Target/ARM/ARMInstrThumb2.td

115

getZExtValue() would have returned the unsigned value, i.e. (uint64_t)2147483648

[rengolin]

Right. LGTM. Thanks!

[efriedma]

llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td

116 ↗

(On Diff #92641)

I think you could simplify this pattern a bit like this:

def t2_so_imm_neg : Operand<i32>, ImmLeaf<i32, [{
  return Imm && ARM_AM::getT2SOImmVal(-(uint32_t)Imm) != -1;
}] ...

lib/Target/ARM/ARMInstrThumb2.td

def t2_so_imm_notSext : Operand<i32>, PatLeaf<(imm), [{
     return ARM_AM::getT2SOImmVal(~N16bitSignExt) != -1;
   }], t2_so_imm_notSext16_XFORM> {
   let ParserMatchClass = t2_so_imm_not_asmoperand;
 }
 
 // t2_so_imm_neg - Match an immediate that is a negation of a t2_so_imm.
 def t2_so_imm_neg_asmoperand : AsmOperandClass { let Name = "T2SOImmNeg"; }
 def t2_so_imm_neg : Operand<i32>, PatLeaf<(imm), [{
-  int64_t Value = -(int)N->getZExtValue();
+  int64_t Value = N->getZExtValue();
+  if (Value == 1LL<<31) return false; // INT_MIN cannot be negated
+  Value = -(int)Value;
   return Value && ARM_AM::getT2SOImmVal(Value) != -1;
 }], t2_so_imm_neg_XFORM> {
   let ParserMatchClass = t2_so_imm_neg_asmoperand;
 }
 
 /// imm0_4095 predicate - True if the 32-bit immediate is in the range [0.4095].
 def imm0_4095_asmoperand: ImmAsmOperand { let Name = "Imm0_4095"; }
 def imm0_4095 : Operand<i32>, ImmLeaf<i32, [{

test/CodeGen/Thumb/ispositive.ll

 ; RUN: llc -mtriple=thumb-eabi %s -o - | FileCheck %s
 
 define i32 @test1(i32 %X) {
 entry:
 ; CHECK-LABEL: test1:
 ; CHECK: lsrs r0, r0, #31
         icmp slt i32 %X, 0              ; <i1>:0 [#uses=1]
         zext i1 %0 to i32               ; <i32>:1 [#uses=1]
         ret i32 %1
 }
 
+define i32 @test2(i32 %X) {
+entry:
+; CHECK-LABEL: test2:
+; CHECK: lsls r1, r1, #31
+; CHECK-NEXT: adds
+        %tmp1 = sub i32 %X, 2147483648
+        ret i32 %tmp1
+}
+