[PATCH] D67398: [DebugInfo] LiveDebugValues: Move DBG_VALUE creation into VarLoc class

[Jeremy Morse via Phabricator via llvm-commits]

jmorse created this revision.
jmorse added reviewers: aprantl, vsk, wolfgangp, djtodoro.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
jmorse added a parent revision: D67393: [DebugInfo] LiveDebugValues: Defer all DBG_VALUE creation during analysis.

In D67393 <https://reviews.llvm.org/D67393> I've described a problem with LiveDebugValues having a feedback effect, which necessitates being able to delete location transfers that are later discovered to be invalid. This patch is an almost-entirely-NFC cleanup (see below for the functional difference) that aims to ease the deletion of transfers in LiveDebugValues.

The main theme here is moving the creation of DBG_VALUE instructions from two different functions into one method in the VarLoc class. A VarLoc and a DBG_VALUE are then effectively equivalent (the DBG_VALUE just describes the contents of a VarLoc), and in a later patch we can delete transfers by VarLoc number, without having to manipulate any DBG_VALUE insts. Reducing duplication has its own virtues too.

In slightly more detail: I've added a couple of factory functions (`CreateCopyLoc` and `CreateEntryLoc`) to encapsulate some slightly fiddly operations ("Use this variable location, but put it in this machine register"). `BuildDbgValue` implements creating a DBG_VALUE from whatever location a VarLoc describes, and condenses the code in `insertTransferDebugPair` and `flushPendingLocs`. A `TransferDebugPair` struct changes from being a `{insert_point, DBG_VALUE}` pair to being a `{insert_point, VarLocID}` pair. Creating a transfer now just means creating a VarLoc and recording the number. Transfer DBG_VALUEs are all created at the end of location propagation via calling `BuildDbgValue`.

There are a couple of subtle artefacts: previously VarLocs could effectively build long chains where the `MI` field referred to the previous transfer DBG_VALUE, eventually rooted in an original source-level assignment DBG_VALUE. Now, however, VarLocs can only ever refer to the originating source-level assignment DBG_VALUE, which I think is slightly neater. Plus, previously a VarLoc was effectively a proxy for a DBG_VALUE inst, to make it easier to identify, but because we use VarLoc as the source for creating DBG_VALUE insts, it now holds real meaningful state (TM). For example, the `Expr` field is the only place an entry-value expression is stored until a DBG_VALUE is created from it.

I'm not familiar with how entry-value information is recorded; as they appear to be register locations with additional expression information, I've recorded them as such.

This patch was going to be 100% NFC, but as it happens it fixes one small problem. Previously if an indirect DBG_VALUE was spilt, when the spill was restored it would be made a direct register reference, losing information along the way. Now, because VarLocs always point their `MI` field at the original source-level assignment DBG_VALUE, this information doesn't get lost. I've added a test function that records this. (Curiously, we can easily get indirect DBG_VALUEs before regalloc, from dbg.declare's of arguments).


Repository:
  rL LLVM

https://reviews.llvm.org/D67398

Files:
  lib/CodeGen/LiveDebugValues.cpp
  test/DebugInfo/MIR/X86/live-debug-values-restore.mir

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67398.219533.patch
Type: text/x-patch
Size: 24109 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190910/234b338e/attachment.bin>