[PATCH] D84653: ARM: make Thumb1 instructions non-flag-setting in IT block.

[Tim Northover via Phabricator via llvm-commits]

t.p.northover updated this revision to Diff 281136.
t.p.northover added a comment.

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).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84653/new/

https://reviews.llvm.org/D84653

Files:
  llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
  llvm/lib/Target/ARM/ARMInstrFormats.td
  llvm/test/CodeGen/ARM/thumb2-it-block.ll
  llvm/test/CodeGen/Thumb2/ifcvt-rescan-diamonds.ll


Index: llvm/test/CodeGen/Thumb2/ifcvt-rescan-diamonds.ll
===================================================================
--- llvm/test/CodeGen/Thumb2/ifcvt-rescan-diamonds.ll
+++ llvm/test/CodeGen/Thumb2/ifcvt-rescan-diamonds.ll
@@ -22,7 +22,8 @@
 ; 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
Index: llvm/test/CodeGen/ARM/thumb2-it-block.ll
===================================================================
--- llvm/test/CodeGen/ARM/thumb2-it-block.ll
+++ llvm/test/CodeGen/ARM/thumb2-it-block.ll
@@ -16,9 +16,7 @@
 
 ; 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
Index: llvm/lib/Target/ARM/ARMInstrFormats.td
===================================================================
--- llvm/lib/Target/ARM/ARMInstrFormats.td
+++ llvm/lib/Target/ARM/ARMInstrFormats.td
@@ -403,8 +403,9 @@
   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;
Index: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
===================================================================
--- llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -537,6 +537,18 @@
     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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84653.281136.patch
Type: text/x-patch
Size: 2752 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200728/e4c129cc/attachment.bin>