Page MenuHomePhabricator

LivePhysRegs: Add support to add pristine registers when populating with live-in/live-out registers.
ClosedPublic

Authored by MatzeB on May 29 2015, 2:44 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 26821.May 29 2015, 2:44 PM
MatzeB retitled this revision from to LivePhysRegs: Add support to add pristine registers when populating with live-in/live-out registers..
MatzeB updated this object.
MatzeB edited the test plan for this revision. (Show Details)
MatzeB set the repository for this revision to rL LLVM.
MatzeB added a subscriber: Unknown Object (MLST).

Hi Matthias,

I miss something.
Why do we need to account for the pristine registers?

Thanks,
-Quentin

Hi Matthias,

I miss something.
Why do we need to account for the pristine registers?

Before prologue/epilogue insertion we don't need to account for them. All current users for LivePhysRegs are before prologue/epilogue insertion so they won't need this (by default AddPristineRegs is false).

After prologue/epilogue insertion those registers are live, you cannot use them for temporary values or place an instruction that would clobber them. Now you can either include them in the set of live registers compute by this class, the alternative is that each time you ask LivePhysReg whether something is alive and you get a negative answer that it may be pristine registers and alive anyway. I decided that it less error prone to simply include the pristine registers in the live registers list.

Hi Matthias,

I miss something.
Why do we need to account for the pristine registers?

Before prologue/epilogue insertion we don't need to account for them. All current users for LivePhysRegs are before prologue/epilogue insertion so they won't need this (by default AddPristineRegs is false).

Actually I just realized that the StackMapLiveness pass uses LivePhysRegs after prologue/epilogue insertion; At a first glance I'd say this is a bug and will miss the pristine registers (though I'm not sure what the repercussions are if pristine regs are missed in patchpoint instruction (or maybe someone adjust for that somewhere else in the code).

I agree, the error proneness is a good argument!

Small comments inlined.

Cheers,
-Quentin

lib/CodeGen/LivePhysRegs.cpp
140 ↗(On Diff #26821)

Is there a reason we are not using MachineFrameInfo::getPristineRegs?

I may be wrong, but at first glance it seems as if we copied/pasted the code from there.

MatzeB added inline comments.Jun 24 2015, 1:23 PM
lib/CodeGen/LivePhysRegs.cpp
140 ↗(On Diff #26821)

This is mostly because the existing API is clunky: getPristineRegs() returns a BitVector by value(!) and has to compute an additional intermediate bitvector which we can avoid here by clearing and setting the registers directly in the LivePhysRegs.

qcolombet accepted this revision.Jun 30 2015, 1:57 PM
qcolombet added a reviewer: qcolombet.

LGTM.

Thanks Matthias!

Q.

This revision is now accepted and ready to land.Jun 30 2015, 1:57 PM
This revision was automatically updated to reflect the committed changes.