[PATCH] D118663: [AArch64] Adds SUBS and ADDS instructions to the MIPeepholeOpt.

[Micah Weston via Phabricator via llvm-commits]

red1bluelost marked 3 inline comments as done.
red1bluelost added a comment.

In D118663#3300642 <https://reviews.llvm.org/D118663#3300642>, @dmgreen wrote:

> Someone once wrote examineCFlagsUse for doing something very similar in the optimizeCompareInstr (because these class as compare instructions). Can some of that be re-used here?
>
> The alternative might be to scan backward through instructions in the main loop, keeping a track of which parts of NZCV are "alive" as we go, so we always have that info available.

Thanks for the suggestion! I was able to use `examineCFlagsUse`. I had to give it external linkage so that I could access it in MIPeepholeOpt. I also made it general to any NZCV case which required slight changes to the preexisting call sites.



================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:308
+         "Instruction must only implicit define NZCV");
+  Register NZCV = MI.implicit_operands().begin()->getReg();
+  for (MachineBasicBlock::iterator Iter = std::next(MI.getIterator()),
----------------
dmgreen wrote:
> Will this always be AArch64::NZCV?
`examineCFlagsUse` makes this no longer relevant.


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:343
+    // Instruction defined NZCV so we can stop scanning
+    if (WritesNZCV)
+      break;
----------------
dmgreen wrote:
> We can stop scanning as soon as the NZCV operand is marked as Dead too, I think.
`examineCFlagsUse` makes this no longer relevant.


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:346
+  }
+  return true;
+}
----------------
dmgreen wrote:
> On rare occasion the NZCV might be live-out of the block (which is probably enough reason to just return false, as opposed to trying to scan further blocks).
`examineCFlagsUse` takes care of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118663