[PATCH] D62904: [DebugInfo] Honour variable fragments in LiveDebugValues

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

Stylistic nit: unless this will push a lot of lines > 80 columns and kill readability, I would prefer `Fragment` over `Frag`.



================
Comment at: lib/CodeGen/LiveDebugValues.cpp:473
+  // in the pre-computed overlap map, and erase them too.
+  auto MapIt = OverlappingFrags.find(std::make_pair(Var.getVar(), ThisFrag));
+  if (MapIt != OverlappingFrags.end()) {
----------------
does `OverlappingFrags.find({Var.getVar(), ThisFrag});` work, too?


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:478
+      LiveDebugValues::OptFragInfo FragHolder =
+          (IsDefault) ? None : LiveDebugValues::OptFragInfo(Fragment);
+      LiveDebugValues::DebugVariable NewFrag(Var.getVar(), FragHolder,
----------------
Perhaps
```
LiveDebugValues::OptFragInfo FragHolder;
if (!DebugVariable::isFragDefault(Fragment))
  FragHolder = LiveDebugValues::OptFragInfo(Fragment);
```


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:481
+                                             Var.getInlinedAt());
+      DoErase(NewFrag);
+    }
----------------
does `DoErase({Var.getVar(), FragHolder, Var.getInlinedAt()})` work?


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:887
+///           Var/Fragment pair to a vector of fragments known to overlap.
+void LiveDebugValues::accumulateFragMap(MachineInstr &MI,
+                                        VarToFrags &SeenFragments,
----------------
FragmentMap?


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:919
+    // Does this previously seen fragment overlap?
+    if (DIExpression::fragmentCmp(ThisFrag, AFrag) == 0) {
+      // Yes: Mark the current fragment as being overlapped.
----------------
Should we have a wrapper for the == 0 case that contains the word overlap in the name?


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

https://reviews.llvm.org/D62904