[Thumb1] The recently added tADCS and tSBCS pseudo-instructions were missing `Uses = [CPSR]`

[tyomitch]

Summary

Thanks to Oliver Stannard for helping catch this.

[efriedma]

test/CodeGen/Thumb/long.ll

234 ↗

(On Diff #94540)

Please write an MIR testcase; it's difficult to understand what this is testing.

[tyomitch]

test/CodeGen/Thumb/long.ll

234 ↗

(On Diff #94540)

The issue in this case was that the missing implicit-use of CPSR on tADCS led to the last tADDi8 being treated as dead, and eliminated (apparently by TwoAddressInstructionPass). The pass itself was doing everything right, it's just that its input was wrong.

I'm not sure what kind of MIR would make this test case easier to understand.

[efriedma]

test/CodeGen/Thumb/long.ll

234 ↗

(On Diff #94540)

For someone reading it, it isn't obvious why we're generating adcs in the first place (and if we improve constant hoisting at some point to split 64-bit constants, the adcs will go away). So I'd prefer an MIR test checking that TwoAddressInstructionPass or whatever isn't eliminating the tADDi8.

If you'd prefer to stick with an IR test, please change it so it doesn't depend on constant hoisting.

[efriedma]

Ping.

[tyomitch]

Sorry for the long delay! Test case updated to make the long adds explicit, and not depend on constant hoisting.

[efriedma]

LGTM, with one minor comment.

test/CodeGen/Thumb/long.ll

229 ↗

(On Diff #95989)

"; CHECK-NEXT: adcs", so nothing slips between.

[tyomitch]

test/CodeGen/Thumb/long.ll

229 ↗

(On Diff #95989)

There's actually push {r4}; pop {r1} between these two; but the other two aren't separated, so I'll change the other CHECK: adcs to CHECK-NEXT.

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

T1sIGenEncode<0b01100, (outs tGPR:$Rd), (ins tGPR:$Rn, tGPR:$Rm),
                   [(set tGPR:$Rd, (add tGPR:$Rn, tGPR:$Rm))]>, Sched<[WriteALU]>;
 
   /// Similar to the above except these set the 's' bit so the
   /// instruction modifies the CPSR register.
   ///
   /// These opcodes will be converted to the real non-S opcodes by
   /// AdjustInstrPostInstrSelection after giving then an optional CPSR operand.
   let hasPostISelHook = 1, Defs = [CPSR] in {
-    let isCommutable = 1 in
+    let isCommutable = 1, Uses = [CPSR] in
     def tADCS : tPseudoInst<(outs tGPR:$Rdn), (ins tGPR:$Rn, tGPR:$Rm),
                             2, IIC_iALUr,
                             [(set tGPR:$Rdn, CPSR, (ARMadde tGPR:$Rn, tGPR:$Rm,
                                                             CPSR))]>,
                 Requires<[IsThumb1Only]>,
                 Sched<[WriteALU]>;
 
     def tADDSi3 : tPseudoInst<(outs tGPR:$Rd), (ins tGPR:$Rm, imm0_7:$imm3),
T1sIGenEncode<0b01101, (outs tGPR:$Rd), (ins tGPR:$Rn, tGPR:$Rm),
                 Sched<[WriteALU]>;
 
 /// Similar to the above except these set the 's' bit so the
 /// instruction modifies the CPSR register.
 ///
 /// These opcodes will be converted to the real non-S opcodes by
 /// AdjustInstrPostInstrSelection after giving then an optional CPSR operand.
 let hasPostISelHook = 1, Defs = [CPSR] in {
+  let Uses = [CPSR] in
   def tSBCS : tPseudoInst<(outs tGPR:$Rdn), (ins tGPR:$Rn, tGPR:$Rm),
                           2, IIC_iALUr,
                           [(set tGPR:$Rdn, CPSR, (ARMsube tGPR:$Rn, tGPR:$Rm,
                                                           CPSR))]>,
               Requires<[IsThumb1Only]>,
               Sched<[WriteALU]>;
 
   def tSUBSi3 : tPseudoInst<(outs tGPR:$Rd), (ins tGPR:$Rm, imm0_7:$imm3),

llvm/trunk/test/CodeGen/Thumb/long.ll

entry:
         ret i64 %tmp1
 ; CHECK-LABEL: f12:
 ; CHECK: movs r2, #1
 ; CHECK: lsls r2, r2, #31
 ; CHECK: movs r3, #0
 ; CHECK: adds r0, r0, r2
 ; CHECK: sbcs r1, r3
 }
+
+declare void @f13(i64 %x)
+
+define void @f14(i1 %x, i64 %y) #0 {
+; CHECK-LABEL: f14:
+entry:
+  %a = add i64 %y, 47
+  call void @f13(i64 %a)
+; CHECK: bl
+  br i1 %x, label %if.end, label %if.then
+
+if.then:
+  call void @f13(i64 %y)
+; CHECK: bl
+  br label %if.end
+
+if.end:
+  %b = add i64 %y, 45
+  call void @f13(i64 %b)
+; CHECK: adds
+; CHECK: adcs
+; CHECK: bl
+  %c = add i64 %y, 47
+  call void @f13(i64 %c)
+; CHECK: adds
+; CHECK-NEXT: adcs
+; CHECK: bl
+  ret void
+}
+
+attributes #0 = { optsize }