[PATCH] D46315: [RegUsageInfoCollector] Fix handling of callee saved registers with CSR optimization.

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added a comment.

> The default implementation should include the aliases (it relies on isPhysRegModified).

I am confused now:

This loop (per what determineCalleeSaves does) will only add any of the registers that getCalleeSavedRegs() returns:

  const MCPhysReg *CSRegs = MF.getRegInfo().getCalleeSavedRegs();
  for (unsigned i = 0; CSRegs[i]; ++i) {
    unsigned Reg = CSRegs[i];
    if (CallsUnwindInit || MRI.isPhysRegModified(Reg))
      SavedRegs.set(Reg);
  }

MRI.isPhysRegModified(Reg) checks if any aliases of Reg is modified, but then this only *inserts* Reg.

SystemZRegisterInfo::getCalleeSavedRegs basically returns CSR_SystemZ_SaveList which is:

  def CSR_SystemZ : CalleeSavedRegs<(add (sequence "R%dD", 6, 15),
                                         (sequence "F%dD", 8, 15))>;

and generated as:

  static const MCPhysReg CSR_SystemZ_SaveList[] = { SystemZ::R6D, SystemZ::R7D, SystemZ::R8D, SystemZ::R9D, SystemZ::R10D, SystemZ::R11D, SystemZ::R12D, SystemZ::R13D, SystemZ::R14D, SystemZ::\
  R15D, SystemZ::F8D, SystemZ::F9D, SystemZ::F10D, SystemZ::F11D, SystemZ::F12D, SystemZ::F13D, SystemZ::F14D, SystemZ::F15D, 0 };

So in fact, it seems that the aliases of a callee saved register is *not* added to SavedRegs on SystemZ by the default implementation.

This works during normal operation because we simply have to save/restore any of these registers if they are clobbered, and there isn't any need to check for any aliases while doing so.

So I am still not sure what to think: Should SystemZ start to add all the aliases into CSR_SystemZ (which seems like unnecessary extra work), or else IPRA would have to do it per my previous question.

IPRA needs the set of fully saved/restored registers while building the regmask of clobbered registers for the whole function. I thought this is the only user of determineCalleeSaves that needs this, and that's why I suggested that this would be done inside the IPRA pass.


https://reviews.llvm.org/D46315