[PATCH] D98564: [AArch64] Peephole rule to remove redundant cmp after cset.

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1536
 
+/// For given instrucion returns operand number which contains condition code,
+/// if instruction is either branch or select, and -1 otherwise.
----------------
Minor nit: Doxygenification


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1594
+  return CCIdx >= 0 ? static_cast<AArch64CC::CondCode>(
+                          Instr.getOperand(CCIdx).getImm())
+                    : AArch64CC::Invalid;
----------------
We may want to assert that this is an immediate.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1742
+///
+/// CmpInstr can be removed if:
+/// - CmpInstr is 'ADDS %vreg, 0' or 'SUBS %vreg, 0',
----------------
ilinpv wrote:
> paquette wrote:
> > I don't think it's necessary to enumerate every case in this comment. It may fall out of sync if someone updates the code.
> I was inspired by canInstrSubstituteCmpInstr doxygen comments. I guess it should be clear what function is doing.  Does it make sense to leave general  doxygen comment for canCmpInstrBeRemoved, but put detailed ordinary comments inside function plus doxygen comments for examineCFlagsUse?
I think that would be good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98564