[PATCH] D158759: [RISCV] Add a pass to rewrite rd to x0 for non-computational instrs whose return values are unused

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 29 12:20:11 PDT 2023


craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVDeadRegisterDefinitions.cpp:65
+    for (MachineInstr &MI : MBB) {
+      // We only handle non-computational instructions since some NOP encodings
+      // are reserved for HINT instructions.
----------------
reames wrote:
> I don't think we need to worry about the reserved nops, as they won't have an MI representation will they?  Unless we've implemented one, and we could just guard explicitly for the reserved nop cases?
I think this was trying to prevent turning a regular instruction like add into a reserved nop encoding? If for some reason dead code elimination didn't already delete a dead add.


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:402
+  }
   addPass(createRISCVInsertVSETVLIPass());
   addPass(createRISCVInsertReadWriteCSRPass());
----------------
reames wrote:
> If you place your new pass after InsertVSETVLI, you should be able to delete this block of code from that pass.
> 
> 
> ```
>   // Once we're fully done rewriting all the instructions, do a final pass
>   // through to check for VSETVLIs which write to an unused destination.
>   // For the non X0, X0 variant, we can replace the destination register
>   // with X0 to reduce register pressure.  This is really a generic
>   // optimization which can be applied to any dead def (TODO: generalize).
>   for (MachineBasicBlock &MBB : MF) {
>     for (MachineInstr &MI : MBB) {
>       if (MI.getOpcode() == RISCV::PseudoVSETVLI ||
>           MI.getOpcode() == RISCV::PseudoVSETIVLI) {
>         Register VRegDef = MI.getOperand(0).getReg();
>         if (VRegDef != RISCV::X0 && MRI->use_nodbg_empty(VRegDef))
>           MI.getOperand(0).setReg(RISCV::X0);
>       }
>     }
>   }
> 
> ```
We'd have to remove the `mayLoad` check I think, but then we'd have to block changing PseudoVSETVLIX0?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158759



More information about the llvm-commits mailing list