[PATCH] D58042: [LiveDebugValues] Emit parameter's entry value

Wolfgang Pieb via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 11 18:16:35 PST 2019


wolfgangp added inline comments.


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:296
+                       VarLocMap &VarLocIDs, TransferMap &Transfers,
+                       SmallVector<MachineInstr *, 6> &ParametrEntryVals,
+                       SparseBitVector<> &KillSet);
----------------
I would prefer either the name "ParameterEntryVals" or "ParamEntyrVals", throughout the file, instead of "ParametrEntryVals".


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:312
+               TransferMap &Transfers,
+               SmallVector<MachineInstr *, 6> &DeadCallParams,
+               SmallVector<MachineInstr *, 6> &ParametrEntryVals,
----------------
This type is used in several locations. Perhaps we could have a using declaration
for it and use a more compact name?


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:825
+    for (unsigned ID : OpenRanges.getVarLocs()) {
+      const DICallSiteParam *DCSP = It->getDebugCallSiteParam();
+      const DILocalVariable *DLV = VarLocIDs[ID].Var.getVar();
----------------
This is invariant in the for-loop, no? Perhaps it should be pulled ahead of it?


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:998
+      // location and that should mean that all parameter entry locations are
+      // covered.
+      // Note: We check whether MI is inlined in order to deduce wheter vraible
----------------
I think this comment needs perhaps some rephrasing. Something like:
Enter DEBUG_VALUE instructions that track parameters into ParameterEntryVals. When we encounter a variable that we have already entered, we assume that we have found all parameter
entry locations and stop.


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:1000
+      // Note: We check whether MI is inlined in order to deduce wheter vraible
+      //       that it tracks comes from different fucntion. If that it the
+      //       case we can't track its entry value.
----------------
some spelling errors: wheter -> whether, vraible->variable, it->is

Suggested wording:
We check whether MI is inlined in order to deduce whether the variable that it tracks
comes from a different function. If that is the case we can't track its entry value.


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

https://reviews.llvm.org/D58042





More information about the llvm-commits mailing list