[PATCH] D98564: [AArch64] Peephole rule to remove redundant cmp after cset.
Jessica Paquette via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 23 10:28:01 PDT 2021
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
More information about the llvm-commits
mailing list