[PATCH] D131958: Add all constant physical registers to callee preserved masks

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/include/llvm/Target/Target.td:192
+  // register in architectures such as MIPS)
+  bit isConstant = false;
 }
----------------
One could follow up with a way to TableGen the implementation of isConstantPhysReg


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.td:240
 
-defm SRC_SHARED_BASE : SIRegLoHi16<"src_shared_base", 235>;
-defm SRC_SHARED_LIMIT : SIRegLoHi16<"src_shared_limit", 236>;
-defm SRC_PRIVATE_BASE : SIRegLoHi16<"src_private_base", 237>;
-defm SRC_PRIVATE_LIMIT : SIRegLoHi16<"src_private_limit", 238>;
+defm SRC_SHARED_BASE : SIRegLoHi16Const<"src_shared_base", 235>;
+defm SRC_SHARED_LIMIT : SIRegLoHi16Const<"src_shared_limit", 236>;
----------------
let ... in would avoid the need to mess with the multiclasses but that's a matter of taste for AMDGPU maintainers


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:80
 let RegAltNameIndices = [ABIRegAltName] in {
-  def X0  : RISCVReg<0, "x0", ["zero"]>, DwarfRegNum<[0]>;
+  def X0  : RISCVReg<0, "x0", ["zero"]>, DwarfRegNum<[0]> { let isConstant = true; }
   let CostPerUse = [0, 1] in {
----------------
I don't know about other architectures, but RISCV style would be to use let ... in (like CostPerUse below). It'd be good to make sure your change it style-consistent for each backend.


================
Comment at: llvm/utils/TableGen/CodeGenRegisters.cpp:159
+      SubRegsComplete(false), SuperRegsComplete(false), TopoSig(~0u) {
   Artificial = R->getValueAsBit("isArtificial");
 }
----------------
I would expect Constant and Artificial to be handled the same way but I don't know why this is here...


================
Comment at: llvm/utils/TableGen/RegisterInfoEmitter.cpp:1616
     }
+    // Add all constant physical registers to the preserved mask:
+    SetTheory::RecSet ConstantSet;
----------------
New line before the comment like above


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131958