[PATCH] D62904: [DebugInfo] Honour variable fragments in LiveDebugValues
Adrian Prantl via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 11 09:56:39 PDT 2019
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
More information about the llvm-commits
mailing list