Page MenuHomePhabricator

[DebugInstrRef][1/3] Track PHI values through register allocation
Needs ReviewPublic

Authored by jmorse on Aug 28 2020, 1:21 PM.


Group Reviewers

(Context: the patch series' at D83046 and D85741). This series tracks PHI instructions through register allocation -- which in the original RFC, I wrote:

"My understanding is that the register allocation phase of LLVM starts there [phielim] and ends after virtregrewriter, and it'd be legitimate to say "we do special things for these passes"."

This patch introduces the "special things": in PHI Elimination we record the position of any PHI instruction that is eliminated, its register and debug instruction number. The register is updated through register allocation; and afterwards, we emit a 'DBG_PHI' instruction identifying the physreg / stack slot where the PHI occurs.

An example of the DBG_PHI instruction working would be:

   DBG_PHI $rax, 1
   [... more insts ... ]

The DBG_PHI identifies the position at which the PHI occurs ($rax here) for debug instruction 1, which can then be picked up by instruction-referencing LiveDebugValues, and tracked until a DBG_INSTR_REF instruction uses that debug instruction number.

This may look a bit like a DBG_VALUE, but it isn't: by using two instructions, we've separated two really important parts of variable value tracking:

  • Where the machine value comes from (specified by DBG_PHI),
  • Where the machine value becomes the variables value.

For PHI instructions, we absolutely need a program point to identify the machine value: that's the nature of PHIs. Thus, to track PHI values, we need to put a new instruction to identify where the PHI occurs.

In this particular patch, I've added boilerplate for the DBG_PHI instruction, some data structures in MachineFunction to track PHIs during regalloc, and code in LiveDebugVariables to update locations when virtual registers are split plus emit DBG_PHIs afterwards. In the other two patches:

  • Register coalescing
  • Handling this all in LiveDebugValues

Diff Detail

Event Timeline

jmorse created this revision.Aug 28 2020, 1:21 PM
jmorse requested review of this revision.Aug 28 2020, 1:21 PM
aprantl added inline comments.

nit /// comes before the field, ///< after.


do we need the sorted-ness of the map for determinism, or could we use a DenseMap?


insert({ID, p});

jmorse updated this revision to Diff 290943.Sep 10 2020, 4:55 AM

Address comments; add a test for creating DBG_PHIs for PHIs that happen in spill slots.

(I also suggested a test was needed for sub-register inputs; however on further examination, you can't PHI unequal register classes, so subregisters don't enter the picture at that stage).

jmorse marked 3 inline comments as done.Sep 10 2020, 4:55 AM
dstenb added a subscriber: dstenb.Sep 10 2020, 9:01 AM

Just some drive-by nits while familiarizing myself with this patch series.






Nit: Capital letter and period.

jmorse updated this revision to Diff 290997.Sep 10 2020, 9:09 AM
jmorse marked 2 inline comments as done.

Revise some dodgy comments, cheers!

jmorse edited the summary of this revision. (Show Details)Oct 14 2020, 9:29 AM

ping; (also remove from summary the reference to additional tests needed, as these are now in the patch).