This is an archive of the discontinued LLVM Phabricator instance.

[MIR] Support Customed Register Mask and CSRs
ClosedPublic

Authored by oren_ben_simhon on Mar 15 2017, 2:15 AM.

Details

Summary

The MIR printer dumps a string that describe the register mask of a function.
A static predefined list of register masks matches a static list of strings.

However when the register mask is not from the static predefined list, there is no descriptor string and the printer fails.
This patch adds support to custom register mask printing and dumping.

Also the list of callee saved registers (describing the registers that must be preserved for the caller) might be dynamic.
As such this data needs to be dumped and parsed back to the Machine Register Info.

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB edited edge metadata.Mar 15 2017, 11:29 AM

Thanks for working on this!

  • Printing the regmask as hexnumbers seems like a bad idea: Adding or removing registers in between would render old .mir files invalid. Even though it's unwieldy I think we should print lists of register names instead. Printing the list of preserved registers instead of the clobbered ones may also make the lists shorter.
  • I only very recently learned about the thing that was called "CalleeSavedRegisters" (or UnusedPhysRegs after your patch). It is silly that we serialize this information in the first place as you can trivially recompute it after loading a mir file: Loop over all blocks, instructions, operands and call MachineRegisterInfo::addPhysRegsUsedFromRegMask on every regmask operand you find. That way we have correct information and don't have to deal with printing and parsing this information in the first place. Maybe you could change this as part of this patch?
  • Browsing around I found "@todo Need to add support in MIPrinter and MIParser to represent..." in the middle of X86 code which you can remove with this.
include/llvm/CodeGen/MachineRegisterInfo.h
78–82 ↗(On Diff #91837)

Unrelated and so trivial that you can just commmit this independently without review.

lib/CodeGen/MIRParser/MIParser.cpp
1676 ↗(On Diff #91837)

Writing the type instead of auto is friendlier to readers (unless the type is immediately obvious, which it is not here).

1682 ↗(On Diff #91837)

typo

1683 ↗(On Diff #91837)

Move the declaration to where it is used to avoid the impression that it needs to hold values accross loop iterations.

lib/CodeGen/MIRPrinter.cpp
889–892 ↗(On Diff #91837)

RegMask operands with a nullptr in the masks are illegal and it is perfectly reasonable not to test for that here and just crash in that case.

lib/CodeGen/MachineRegisterInfo.cpp
593 ↗(On Diff #91837)

Use ArrayRef<MCPhysReg> here for increased flexibility (smallvectors are among the things that automatically convert to ArrayRefs).

597 ↗(On Diff #91837)

Please use MCPhysReg instead of auto.

oren_ben_simhon marked 7 inline comments as done.
oren_ben_simhon added a subscriber: aaboud.

Implemented comments posted until 03/15 (Thank you Matthias).

MatzeB accepted this revision.Mar 16 2017, 10:56 AM

LGTM, thanks.

lib/CodeGen/MIRParser/MIRParser.cpp
513–523 ↗(On Diff #91995)

Oh wow, we even already had the code to compute the UsedPhysRegMask :)

This revision is now accepted and ready to land.Mar 16 2017, 10:56 AM
This revision was automatically updated to reflect the committed changes.
llvm/trunk/test/CodeGen/MIR/Generic/dynamic-regmask.ll