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

Tue Sep 5 07:10:12 PDT 2023

asb added a comment.

This may not be much of an issue in practice, but I'd note that rewriting rd to x0 can sometimes result in a non-compressible instruction when previously it was compressible (e.g. rewriting loads).

I agree with the concern about this unwittingly introducing new HINT instructions. Although we expect instructions without side-effects that aren't caught elsewhere to be very rare, wouldn't it make sense to go ahead and delete them in this case? That said, I'm not sure our current modeling of CSR instructions would handle this properly for a theoretical architecture that had side-effecting CSR reads (e.g. a FIFO accessed via a CSR).

================
Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll:12396
 ; RV64ZVE32F-NEXT:    lbu a2, 0(a2)
-; RV64ZVE32F-NEXT:    li a3, 32
+; RV64ZVE32F-NEXT:    li zero, 32
 ; RV64ZVE32F-NEXT:    vmv.s.x v12, a2
----------------
craig.topper wrote:
> reames wrote:
> > craig.topper wrote:
> > > I don't know why it didn't get removed either, but its not ok for this pass to change this instruction.
> > It only became dead after InsertVSETVLI and we don't run DCE after that.
> > 
> > @craig.topper Why isn't this legal?  If the instruction is dead, this does appear to be a valid instruction encoding?  I don't see anything in the spec about addi zero, zero, imm being reserved or anything.  
> Isn't it one of the hint encodings? It won't crash, but it might cause unintended effects from hint. If it was an ORI it could be a non-temporal hint for example. If it was an AUIPC it would become a landing pad from the CFI proposal.
Agreed this is undesirable for HINT instructions. These are standard RV32I instructions that do not alter architectural state, so of course very commonly have rd==x0.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158759