[PATCH] D146734: [CodeGen] Use RegUnits in RegisterClassInfo::getLastCalleeSavedAlias

Tue Apr 4 06:56:07 PDT 2023

arsenm added inline comments.

================
Comment at: llvm/include/llvm/CodeGen/RegisterClassInfo.h:121
+    for (MCRegUnitIterator UI(PhysReg, TRI); UI.isValid(); ++UI) {
+      CSR = CalleeSavedAliases[*UI];
+      if (CSR)
----------------
qcolombet wrote:
> foad wrote:
> > qcolombet wrote:
> > > Cannot this get out of range?
> > > I.e., I believe we could assert that `*UI` is bigger than `CalleeSavedAliases.size()`.
> > > 
> > > I am wondering why the check was there in the previous implementation.
> > I don't know! I just found that when I removed the check, everything still worked.
> Oh boy!
> Looking at the history of the change, I don't see any explanation why we needed that check. (Unless I am mistaken, the check has been introduced with https://reviews.llvm.org/D28566.)
> 
> Could it be possible to call `getLastCalleeSavedAlias` before `CalleeSavedAliases` has been initialized?
My guess would be to handle calling conventions with 0 callee saved registers. Given that would cover the most common case for AMDGPU this is probably ok

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146734