[PATCH] D109076: [RISCV] Use GPRNoX0 for source register to WriteSysReg/SwapSysReg pseudo instructions.

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added reviewers: frasercrmck, jrtc27, asb, luismarques, sepavloff.
Herald added subscribers: StephenFan, vkmr, evandro, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, hiraditya.
craig.topper requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: LLVM.

Having X0 as a source of a CSRRW means don't write the register. To
write a 0, you should use CSRRWI instead. ISel normally takes care of
this. To further guarantee this I've merged the classes so we must
have isel patterns for imm 0 if we have the register operation.

It is possible that a X0 is an operand of a select that feeds the CSR
write pseudo instruction. The select will be expanded to control flow
and phis which later be turned into copies. This can allow register
coalescing to coalesce the X0 into a CSR write pseudo instruction. By
using GPRNoX0 the coalescing should be prevented.

We don't yet have any uses of these pseudo instructions that I know of
so I can't test this. I'm extrapolating from a similar issue we found
on vsetvli for vector instructions. That's a much larger bug to fix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109076

Files:
  llvm/lib/Target/RISCV/RISCVInstrInfo.td


Index: llvm/lib/Target/RISCV/RISCVInstrInfo.td
===================================================================
--- llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -1195,43 +1195,35 @@
   let Uses = Regs;
 }
 
-class WriteSysReg<SysReg SR, list<Register> Regs>
-  : Pseudo<(outs), (ins GPR:$val),
-           [(riscv_write_csr (XLenVT SR.Encoding), GPR:$val)]>,
-    PseudoInstExpansion<(CSRRW X0, SR.Encoding, GPR:$val)> {
-  let hasSideEffects = 0;
-  let Defs = Regs;
-}
-
-class WriteSysRegImm<SysReg SR, list<Register> Regs>
-  : Pseudo<(outs), (ins uimm5:$val),
-           [(riscv_write_csr (XLenVT SR.Encoding), uimm5:$val)]>,
-    PseudoInstExpansion<(CSRRWI X0, SR.Encoding, uimm5:$val)> {
-  let hasSideEffects = 0;
-  let Defs = Regs;
-}
-
-class SwapSysReg<SysReg SR, list<Register> Regs>
-  : Pseudo<(outs GPR:$rd), (ins GPR:$val),
-           [(set GPR:$rd, (riscv_swap_csr (XLenVT SR.Encoding), GPR:$val))]>,
-    PseudoInstExpansion<(CSRRW GPR:$rd, SR.Encoding, GPR:$val)> {
-  let hasSideEffects = 0;
-  let Uses = Regs;
-  let Defs = Regs;
+// Need to use GPRNoX0 for $val to prevent X0 copies coalescing into this
+// causing it to become CSRRW x0, x0 which is a NOP.
+multiclass WriteSysReg<SysReg SR, list<Register> Regs> {
+  let hasSideEffects = 0, Defs = Regs in {
+    def NAME : Pseudo<(outs), (ins GPRNoX0:$val),
+                      [(riscv_write_csr (XLenVT SR.Encoding), GPRNoX0:$val)]>,
+               PseudoInstExpansion<(CSRRW X0, SR.Encoding, GPR:$val)>;
+    def Imm : Pseudo<(outs), (ins uimm5:$val),
+                     [(riscv_write_csr (XLenVT SR.Encoding), uimm5:$val)]>,
+              PseudoInstExpansion<(CSRRWI X0, SR.Encoding, uimm5:$val)>;
+  }
 }
 
-class SwapSysRegImm<SysReg SR, list<Register> Regs>
-  : Pseudo<(outs GPR:$rd), (ins uimm5:$val),
-           [(set GPR:$rd, (riscv_swap_csr (XLenVT SR.Encoding), uimm5:$val))]>,
-    PseudoInstExpansion<(CSRRWI GPR:$rd, SR.Encoding, uimm5:$val)> {
-  let hasSideEffects = 0;
-  let Uses = Regs;
-  let Defs = Regs;
+// Need to use GPRNoX0 for $val to prevent copies from X0 coalescing into this
+// causing it to become CSRRW rd, x0 which will not write the CSR.
+multiclass SwapSysReg<SysReg SR, list<Register> Regs> {
+  let hasSideEffects = 0, Uses = Regs, Defs = Regs in {
+    def NAME : Pseudo<(outs GPR:$rd), (ins GPRNoX0:$val),
+                      [(set GPR:$rd,
+                        (riscv_swap_csr (XLenVT SR.Encoding), GPRNoX0:$val))]>,
+               PseudoInstExpansion<(CSRRW GPR:$rd, SR.Encoding, GPR:$val)>;
+    def Imm : Pseudo<(outs GPR:$rd), (ins uimm5:$val),
+                     [(set GPR:$rd,
+                       (riscv_swap_csr (XLenVT SR.Encoding), uimm5:$val))]>;
+  }
 }
 
 def ReadFRM : ReadSysReg<SysRegFRM, [FRM]>;
-def WriteFRM : WriteSysReg<SysRegFRM, [FRM]>;
-def WriteFRMImm : WriteSysRegImm<SysRegFRM, [FRM]>;
+defm WriteFRM : WriteSysReg<SysRegFRM, [FRM]>;
 
 /// Other pseudo-instructions
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D109076.369993.patch
Type: text/x-patch
Size: 2987 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210901/b2d2bdf2/attachment.bin>