[PATCH] D66412: [DebugInfo][LiveDebugValues] Don't drop variable location tracking data across block boundaries

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:12
+/// instruction stream. Before running, each DBG_VALUE corresponds to a source
+/// assignment of a variable. Afterwards, a DBG_VALUE inst specifies a variable
+/// location for the current basic block (see SourceLevelDebugging.rst).
----------------
consistency between "DBG_VALUE instructions", "DBG_VALUE inst", and "DBG_VALUE" ? :-)


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:332
 
+    /// Insert a set of ranges.
+    void loadFromLocSet(const VarLocSet &ToLoad, const VarLocMap &Map) {
----------------
I find the naming/comment pair confusing.. should this be called insertFromLocSet?


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:400
+  /// Create DBG_VALUE insts for inlocs that have been propagated but
+  /// had their inst creation deferred.
+  void flushPendingLocs(VarLocInMBB &PendingInLocs, VarLocMap &VarLocIDs);
----------------
inst -> instruction?


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:1113
+  // New locations should have DBG_VALUEs inserted at the start of the block.
+  // This creation is now deferred until the end of LiveDebugValues, instead
+  // the new location is recorded in the Pending set.
----------------
The way this comment is written it is better suited for a commit message. It would be better if the comment described what/how the code does, not a change over how it was before.

"When finalizing the LiveDebugValues pass, all new incoming locations are inserted as DBG_VALUEs at the beginning of each block" or something like that


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:1266
+
+    // Initialize pending inlocs
+    PendingInLocs[&MBB] = VarLocSet();
----------------
`.`


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66412/new/

https://reviews.llvm.org/D66412