[PATCH] D45156: [MachineVerifier] Verify the RegUsageInfo collected for the current function.

Jonas Paulsson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 25 03:46:42 PDT 2018


jonpa marked 3 inline comments as done.
jonpa added a comment.

In https://reviews.llvm.org/D45156#1087897, @qcolombet wrote:

> Good catch!
>
> Actually, following the other related patch, I think  we should use determineCalleeSaved. Moreover, I don't remember when this function is supposed to give the right answer, but we probably need to gate the check behind NoVRegs or something like that.


I think it's ok since PRUI is not set until the pass has been run.

I tried to simply reuse the new function from the related patch now, but ran into a problem: A reg with subregs had one subreg completely unclobbered, and the other one saved/restored. We could check subregs again with a new loop to see if they are all saved or unclobbered, or we could just add this extra check in computeCalleeSavedRegs like

  // Add PReg to SavedRegs if all subregs are saved or unmodified.                                                                                                                                                             
  bool AllSubRegsSaved = true;
  for (MCSubRegIterator SR(PReg, TRI, false); SR.isValid(); ++SR)
    if (!SavedRegs.test(*SR) && MRI->isPhysRegModified(*SR)) {
      AllSubRegsSaved = false;
      break;
    }

I am however having second thoughts about verifying this in the MachineVerifier, given that the Collector pass is run after all else (at least after post-RA pseudo expansion and scheduling), so it seems that we are just verifying our own algorithm without having any real benefit of verifying other passes.

Do you agree we could drop this patch?


https://reviews.llvm.org/D45156





More information about the llvm-commits mailing list