[PATCH] D125564: Teach PeepholeOpt to eliminate redundant copy from constant physreg (e.g VLENB on RISCV)

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

This looks ok to me, but I'd like to understand some things about the larger roadmap here.

If SelectionDAG CSEd the read_register this test case wouldn't need this patch. The peephole pass code here seems to be restricted to a single basic block, same as SelectionDAG. read_register isn't CSEd in SelectionDAG because read_register and read_volatile_register use the same code and have a chain dependency.

The main places we create PseudoVLENB are from ISD::VSCALE lowering and frame lowering.

For ISD::VSCALE lowering we go through RISCVISD::READ_VLENB. Do you plan to replace with RISCVISD::READ_VLENB with ISD::READ_REGISTER? Or make RISCVISD::READ_VLENB isel to a COPY? Or maybe use ISD::CopyFromReg? RISCVISD::READ_VLENB and ISD::CopyFromReg should CSE as long as the chain input is EntryNode and there is no Glue.

For the frame lowering case, that happens after regalloc so this patch doesn't help with that.

Do you have other cases that this patch helps that wouldn't already be covered by SelectionDAG CSE?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125564