[PATCH] D63419: [ARM] Thumb2CondExecution. NFC

Sjoerd Meijer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 17 07:14:49 PDT 2019


SjoerdMeijer marked 2 inline comments as done.
SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Target/ARM/Thumb2CondExecution.cpp:103
+           Subreg.isValid(); ++Subreg)
+        Uses.insert(*Subreg);
+  };
----------------
Oopsy daisy:  'Uses' is wrong here, which I will fix.
This passed tests, indicating some tests are missing here...


================
Comment at: llvm/lib/Target/ARM/Thumb2CondExecution.cpp:109
-      Defs.insert(*Subreg);
-    if (Reg == ARM::CPSR)
-      continue;
----------------
I accidentally cleaned this up, i.e. didn't notice it. But it turns out I was lucky: this if and continue doesn't make any sense.


================
Comment at: llvm/lib/Target/ARM/Thumb2CondExecution.cpp:194
 
-bool Thumb2ITBlockPass::InsertITInstructions(MachineBasicBlock &MBB) {
+bool Thumb2ITBlock::InsertITInstructions(MachineBasicBlock &Block) {
   bool Modified = false;
----------------
samparker wrote:
> Just a bike shedding nit... MBB is the defacto variable name for a MachineBasicBlock.. and the same for the iterator names.
In general I like short variable names, but here all permutations MI, MIB, MBBI, MBB, NMI got a bit too much for me. But as I don't plan to touch this part, I will revert these changes.


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

https://reviews.llvm.org/D63419





More information about the llvm-commits mailing list