[PATCH] D133103: [PowerPC] Improve kill flag computation and add verification after MI peephole
Amy Kwan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 23 17:43:46 PST 2022
amyk added a comment.
A few more submitted comments that I realized I had left on the review.
================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3638
+ // Conservatively add defs from DefMI and uses from MI to the set of
+ // registers that need their kill flags updated.
----------------
We should clarify that for `MI`, we're actually adding both the defs and the uses.
================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3642
+ if (MO.isReg() && MO.isDef())
+ UpdatedRegs.push_back(MO.getReg());
+ for (const MachineOperand &MO : MI.operands())
----------------
It would be good to have a more descriptive name for `UpdateRegs`. Perhaps `RegsToUpdate`.
================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:439
+ SmallVector<Register, 2> UpdatedRegs;
+ if (TII->convertToImmediateForm(MI, UpdatedRegs)) {
+ for (Register R : UpdatedRegs)
----------------
Since you're adding a significant amount of code to a deeply nested block, consider flipping this condition to an early exit to reduce the nesting.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133103/new/
https://reviews.llvm.org/D133103
More information about the llvm-commits
mailing list