Page MenuHomePhabricator

MachineFrameInfo: Simplify pristine register calculation.
ClosedPublic

Authored by MatzeB on May 28 2015, 12:47 PM.

Details

Summary

About pristine regsiters:
Pristine registers "hold a value that is useless to the current
function, but that must be preserved - they are callee saved registers
that have not been saved." This concept saves compile time as it frees
the prologue/epilogue inserter from adding every such register to every
basic blocks live-in list.

However the current code in getPristineRegs is formulated in a
complicated way: Inside the function prologue and epilogue all callee
saves are considered pristine, while in the rest of the code only the
non-saved ones are considered pristine. This requires logic to
differentiate between prologue/epilogue and the rest and in the presence
of shrink-wrapping this even becomes complicated/expensive. It's also
unnecessary because the prologue epilogue inserters already mark
callee-save registers that are saved/restores properly in the respective
blocks in the prologue/epilogue (see updateLiveness() in
PrologueEpilogueInserter.cpp). So only declaring non-saved/restored
callee saved registers as pristine just works.

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 26725.May 28 2015, 12:47 PM
MatzeB retitled this revision from to MachineFrameInfo: Simplify pristine register calculation..
MatzeB updated this object.
MatzeB edited the test plan for this revision. (Show Details)
MatzeB added a reviewer: qcolombet.
MatzeB set the repository for this revision to rL LLVM.
MatzeB added a subscriber: Unknown Object (MLST).
qcolombet accepted this revision.May 28 2015, 3:04 PM
qcolombet edited edge metadata.

Hi Matthias,

It's also unnecessary because the prologue epilogue inserters already mark callee-save registers that are saved/restores properly in the respective blocks in the prologue/epilogue.

This assumes that everybody uses the live-in information.

That being said, the new semantic works for me. Just make sure to send a “Heads-up” email to llvm-dev to emphasize the change for out-of-tree targets.

The bottom line LGTM.

Thanks,
-Quentin

This revision is now accepted and ready to land.May 28 2015, 3:04 PM

Thanks for the review.

I think heads-up message is not necessary: The previous semantics just allowed us to have fewer registers in the live-in sets in theory. There is no change for code that does not rely on live-in sets. For code relying on the live-in sets we won't have a change as well because the live-in sets are not changed by this commit.

This revision was automatically updated to reflect the committed changes.