[PATCH] D34220: [AArch64] Prefer B.cond to CBZ/CBNZ/TBZ/TBNZ when NZCV flags can be set for "free"

[Geoff Berry via Phabricator via llvm-commits]

gberry added inline comments.


================
Comment at: lib/Target/AArch64/AArch64CondBrTuning.cpp:76
+void AArch64CondBrTuning::getAnalysisUsage(AnalysisUsage &AU) const {
+  MachineFunctionPass::getAnalysisUsage(AU);
+}
----------------
You should be able to at least do AU.setPreservesCFG() here, otherwise there's no need to override this to just call the parent version.


================
Comment at: lib/Target/AArch64/AArch64CondBrTuning.cpp:81
+  MachineInstr *DefInstr = nullptr;
+  if (MO.isReg() && TargetRegisterInfo::isVirtualRegister(MO.getReg()))
+    DefInstr = MRI->getUniqueVRegDef(MO.getReg());
----------------
The check for MO.isReg() seems unnecessary.  MO.getReg() will assert if it isn't a reg, which it always should be. 
You could simplify this a bit to be:

```
if (!isvirt(MO.getReg())
  return null;
return getUniqueVRegDef(MO.getReg())
```



================
Comment at: lib/Target/AArch64/AArch64CondBrTuning.cpp:190
+  // reads NZCV.
+  MachineBasicBlock::iterator I(DefMI), E(MI);
+  for (I = std::next(I); I != E; ++I) {
----------------
You should probably do this after the opcode check below to save compile time.


https://reviews.llvm.org/D34220