[PATCH] D130919: [MRI] isConstantPhysReg should also check if the register is clobbered by a RegMask

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 1 11:10:43 PDT 2022


arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineRegisterInfo.cpp:583
+void MachineRegisterInfo::collectUsedPhysRegMask() {
+  for (auto MBBI = MF->begin(), MBBE = MF->end(); MBBI != MBBE; ++MBBI)
+    for (MachineInstr &MI : MBBI->instrs())
----------------
This can just be a range loop. Also braces


================
Comment at: llvm/lib/CodeGen/RegisterCoalescer.cpp:4116
   MRI = &fn.getRegInfo();
+  MRI->collectUsedPhysRegMask();
   const TargetSubtargetInfo &STI = fn.getSubtarget();
----------------
Adding a specific collection step here feels wrong. Other contexts the mask is added as soon as it's created. Not sure why we have the virtregrewriter adding these now


================
Comment at: llvm/lib/Target/AMDGPU/SIPreAllocateWWMRegs.cpp:104
   for (MCRegister PhysReg : RegClassInfo.getOrder(MRI->getRegClass(Reg))) {
-    if (!MRI->isPhysRegUsed(PhysReg) &&
+    if (!MRI->isPhysRegUsed(PhysReg, true) &&
         Matrix->checkInterference(LI, PhysReg) == LiveRegMatrix::IK_Free) {
----------------
Don't see why this would skip regmask tests


================
Comment at: llvm/lib/Target/Mips/MipsRegisterInfo.cpp:322-323
+
+bool MipsRegisterInfo::isConstantPhysReg(MCRegister PhysReg) const {
+  return PhysReg == Mips::ZERO_64 || PhysReg == Mips::ZERO;
+}
----------------
This should be a separate mips change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130919



More information about the llvm-commits mailing list