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

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

- The transformation make sense to me.
- Does this need to be a separate pass? Glancing at the code, it seems that performBRCONDCombine() in AArch64ISelLowering.cpp is the only place creating CBZ instructions (except for FastISel). So maybe the adjustment can be performed there?
- The code has a lot of opcode "tables" (in the form of switch/case). It's a judgement call each time, but generally I think it looks better if we have most opcode based tables in AArch64InstrInfo. At least the part mapping "S" opcodes to the non-"S" opcodes looks like a good candidate.



================
Comment at: lib/Target/AArch64/AArch64CondBrTuning.cpp:10-25
+// This file contains a pass that transforms CBZ/CBNZ/TBZ/TBNZ instructions into
+// a conditional branch (B.cond), when the NZCV flags can be set for "free".
+// This is preferred on targets that have more flexibility when scheduling
+// B.cond instructions as compared to CBZ/CBNZ/TBZ/TBNZ (assuming all other
+// variables are equal).  This can also reduce register pressure.
+//
+// A few examples:
----------------
You should use `/// \file` at the beginning and continue with `///` so that doxygen can pick it up.


================
Comment at: lib/Target/AArch64/AArch64CondBrTuning.cpp:334
+  bool Changed = false;
+  for (auto &MBB : MF) {
+    bool LocalChange = false;
----------------
Writing `MachineBasicBlock &` instead of `auto &` would be friendlier to the reader.


https://reviews.llvm.org/D34220