[PATCH] D124118: [Peephole-Opt] For one kind of test-after-add pattern, eliminates test if it's correct to do so.

[Kan Shengchen via Phabricator via llvm-commits]

skan added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:995
+      (MRI->getVRegDef(CmpInstr.getOperand(0).getReg()) == &CmpValDefInstr) &&
+      "Caller guaranteed.");
+
----------------
What does the "Caller guranteed" mean here?  Does it say that `TEST64rr` is the user of `SUBREG_TO_REG`?


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:1031
+    // be optimized away.
+    for (MachineInstr &Instr :
+         make_range(std::next(MachineBasicBlock::iterator(VregDefInstr)),
----------------
const MachineInstr &


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:970
 
+inline static bool
+findRedundantFlagInstr(MachineInstr &CmpInstr, MachineInstr &CmpValDefInstr,
----------------
mingmingl wrote:
> skan wrote:
> > Remove `inline`?
> Removed `inline`.
> 
> For my understanding, is it because of lengthy function body and long argument list that `inline` hint doesn't make much sense here?
Yes.  In fact, I think it's useless to add `inline` to functions w/ internal linkage for non-peformance critical code. Compiler can decide whether it should be inlined.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124118