[PATCH] D147982: Account for PATCHABLE instrs in Branch Relaxation

Jessica Paquette via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 11 11:17:10 PDT 2023


paquette added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetInstrInfo.cpp:1653
   // expands to special code sequences which must be present.
   auto First = MBB.getFirstNonDebugInstr();
+  if (First == MBB.end())
----------------
dhoekwater wrote:
> paquette wrote:
> > I think this could be a `llvm::find_if`?
> > 
> > ```
> > return find_if(MBB.getFirstNonDebugInstr(), MBB.getLastNonDebugInstr(), [](const MachineInstr &MI) {
> >    unsigned Opc = MI.getOpcode();
> >    switch(Opc) {
> >       default:
> >           return false;
> >       case TargetOpcode::FENTRY_CALL:
> >       ...
> >          return true;
> >    }
> > }) != MBB.end();
> > ```
> > 
> > That'd make it easy to add new opcodes/remove opcodes.
> It definitely could be, and it makes the function visually a lot cleaner. Because PATCHABLE instructions are always inserted at the start or at/just before the end, the previous and current implementations are O(1) in the number of instructions.
> 
> Being fairly unfamiliar with the performance characteristics of the codebase, I was hesitant to scan through the whole basic block. If the small performance difference isn't a concern, I'd be happy to switch to `llvm::find_if`.
Ah I see. Yeah let's keep it like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147982



More information about the llvm-commits mailing list