ARM: make Thumb1 instructions non-flag-setting in IT block.

[t.p.northover]

Summary

Many Thumb1 instructions are defined to set CPSR if executed outside an IT block, but leave it alone from inside one. In MachineIR this is represented by whether an optional register is CPSR or NoReg (0), and affects how the instructions are printed.

This sets the instruction to the appropriate form during if-conversion.

[dmgreen]

This looks familiar, as something @NickGuy was looking at recently from D83667. We didn't know about ThumbArithFlagSetting though, that's a nice way to do it.

Can you update the comment from the CodeGen/ARM/thumb2-it-block.ll test too? The code here looks OK to me.

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

547

It sounds OK to search, but will this always just be operand 1?

[t.p.northover]

Thanks for the suggestions and context . Glad it's not quite a direct duplicate of the other one but related.

I've updated the test, and switched to asserting that operand 1 is the one we want because I really can't see it ever being anything different (it would need an instruction to define two registers which just doesn't happen with 16-bit Thumb, or zero and that's basically just cmp which isn't affected).

[dmgreen]

Thanks. LGTM

[t.p.northover]

Thanks. Pushed as 39108f4c7a.

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

if (isUncondBranchOpcode(Opc)) {
     return true;
   }
 
   int PIdx = MI.findFirstPredOperandIdx();
   if (PIdx != -1) {
     MachineOperand &PMO = MI.getOperand(PIdx);
     PMO.setImm(Pred[0].getImm());
     MI.getOperand(PIdx+1).setReg(Pred[1].getReg());
+
+    // Thumb 1 arithmetic instructions do not set CPSR when executed inside an
+    // IT block. This affects how they are printed.
+    const MCInstrDesc &MCID = MI.getDesc();
+    if (MCID.TSFlags & ARMII::ThumbArithFlagSetting) {
+      assert(MCID.OpInfo[1].isOptionalDef() && "CPSR def isn't expected operand");
+      assert((MI.getOperand(1).isDead() ||
+              MI.getOperand(1).getReg() != ARM::CPSR) &&
+             "if conversion tried to stop defining used CPSR");
+      MI.getOperand(1).setReg(ARM::NoRegister);
+    }
+
     return true;
   }
   return false;
 }
 
 bool ARMBaseInstrInfo::SubsumesPredicate(ArrayRef<MachineOperand> Pred1,
                                          ArrayRef<MachineOperand> Pred2) const {
   if (Pred1.size() > 2 || Pred2.size() > 2)

llvm/lib/Target/ARM/ARMInstrFormats.td

class InstTemplate<AddrMode am, int sz, IndexMode im,
   IndexMode IM = im;
   bits<2> IndexModeBits = IM.Value;
   Format F = f;
   bits<6> Form = F.Value;
   Domain D = d;
   bit isUnaryDataProc = 0;
   bit canXformTo16Bit = 0;
   // The instruction is a 16-bit flag setting Thumb instruction. Used
-  // by the parser to determine whether to require the 'S' suffix on the
-  // mnemonic (when not in an IT block) or preclude it (when in an IT block).
+  // by the parser and if-converter to determine whether to require the 'S'
+  // suffix on the mnemonic (when not in an IT block) or preclude it (when
+  // in an IT block).
   bit thumbArithFlagSetting = 0;
 
   bit validForTailPredication = 0;
   bit retainsPreviousHalfElement = 0;
   bit horizontalReduction = 0;
   bit doubleWidthResult = 0;
 
   // If this is a pseudo instruction, mark it isCodeGenOnly.

llvm/test/CodeGen/ARM/thumb2-it-block.ll

  %sub2 = sub nsw i32 0, %b
  %abs2 = select i1 %cmp2, i32 %sub2, i32 %b
  %add = add nsw i32 %abs1, %abs2
  ret i32 %add
 }
 
 ; CHECK:        cmp
 ; CHECK-NEXT:   it    mi
-; We shouldn't need to check for the extra 's' here; tRSB should be printed as
-; "rsb" inside an IT block, not "rsbs".
-; CHECK-NEXT:   rsb{{s?}}mi
+; CHECK-NEXT:   rsbmi
 ; CHECK-NEXT:   cmp
 ; CHECK-NEXT:   it    mi
 ; CHECK-NEXT:   rsb{{s?}}mi
 
 

llvm/test/CodeGen/Thumb2/ifcvt-rescan-diamonds.ll

while.cond38:                                     ; preds = %if.end111, %entry
   br i1 %a, label %cond.true77, label %cond.false87
 
 ; CHECK: %cond.true77
 ; CHECK-NEXT: @ in Loop
 ; CHECK-NEXT: cmp.w {{r[0-9]+}}, #0
 ; CHECK-NEXT: it eq
 ; CHECK-NEXT: ldreq
 ; CHECK-NEXT: it ne
-; CHECK-NEXT: movsne
+  ; N.b. 16-bit mov instruction in IT block does not set flags.
+; CHECK-NEXT: movne
 ; CHECK-NEXT: mvns
 ; CHECK-NEXT: b
 cond.true77:                                      ; preds = %while.cond38
   br i1 %cmp79, label %cond.end84, label %cond.false81
 
 cond.false81:                                     ; preds = %cond.true77
   %0 = load i32, i32* null, align 4
   br label %cond.end84