[ARM] Ensure we update the correct flags in the peephole optimiser

[dmgreen]

Summary

The Arm peephole optimiser code keeps track of both an MI and a SubAdd that can be
used to optimise away a CMP. In the rare case that both are found and not ruled-out as
valid, we could end up setting the flags on the wrong one.

Instead make sure we are using SubAdd if it exists, as it will be closer to the CMP.

The testcase here is a little theoretical, with a dead def of cpsr. It should hopefully show
the point.

[SjoerdMeijer]

LGTM, with one nit inline.
Perhaps wait a day in case someone else has comments.

lib/Target/ARM/ARMBaseInstrInfo.cpp

2828 ↗

(On Diff #186640)

You've already put it in quotes, but perhaps you can describe closer a bit better.

[dmgreen]

Update comment

llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.cpp

if (Instr.modifiesRegister(ARM::CPSR, TRI) ||
       return false;
 
   } while (I != B);
 
   // Return false if no candidates exist.
   if (!MI && !SubAdd)
     return false;
 
-  // The single candidate is called MI.
-  if (!MI) MI = SubAdd;
+  // If we found a SubAdd, use it as it will be closer to the CMP
+  if (SubAdd) {
+    MI = SubAdd;
+    IsThumb1 = false;
+  }
 
   // We can't use a predicated instruction - it doesn't always write the flags.
   if (isPredicated(*MI))
     return false;
 
   // Scan forward for the use of CPSR
   // When checking against MI: if it's a conditional code that requires
   // checking of the V bit or C bit, then this is not safe to do.

llvm/trunk/test/CodeGen/Thumb2/peephole-addsub.mir

+# RUN: llc -run-pass=peephole-opt -verify-machineinstrs -o - %s | FileCheck %s
+--- |
+  target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+  target triple = "thumbv7-none-eabi"
+
+  define i32 @test(i32 %a, i32 %b) {
+    unreachable
+  }
+
+...
+---
+name:            test
+tracksRegLiveness: true
+liveins:
+  - { reg: '$r0', virtual-reg: '%0' }
+  - { reg: '$r1', virtual-reg: '%1' }
+body:             |
+  bb.0 (%ir-block.0):
+    liveins: $r0, $r1
+
+    %1:rgpr = COPY $r1
+    %0:rgpr = COPY $r0
+    %2:rgpr = t2MOVi 1, 14, $noreg, $noreg
+    %3:gprnopc = t2ADDrr %0, %1, 14, $noreg, $noreg
+    %4:gprnopc = t2SUBri %3, 0, 14, $noreg, def dead $cpsr
+    t2CMPri killed %3, 0, 14, $noreg, implicit-def $cpsr
+    %5:rgpr = t2MOVCCi %2, 0, 7, $cpsr
+    $r0 = COPY %5
+    tBX_RET 14, $noreg, implicit $r0
+
+# CHECK-LABEL: name: test
+# CHECK:      %3:gprnopc = t2ADDrr %0, %1, 14, $noreg, $noreg
+# CHECK-NEXT: %4:gprnopc = t2SUBri %3, 0, 14, $noreg, def $cpsr
+# CHECK-NEXT: %5:rgpr = t2MOVCCi %2, 0, 7, $cpsr
+...