[PATCH] D58044: [DwarfDebug] Dump call site debug info into DWARF

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro marked 2 inline comments as done.
djtodoro added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp:39
   assert(MI.getNumOperands() == 4);
+  if (MI.getDebugExpression()->isEntryValue())
+    return 0;
----------------
dstenb wrote:
> Although it is a short function which is easy to skim through, I think it would be helpful to somehow signal that it only applies for non-entry values, e.g. through amending the comment or changing the name (although I don't have any suggestions for the latter).
> 
> Alternatively, maybe the function can be left as it was before this patch, and instead the calls to `{add,drop}RegDescribedVar` can be wrapped with that conditional?
Thanks! Definitely we should change something here.


================
Comment at: lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp:71
+  // Entry value register references can't be clobbered.
+  if (Ranges.back().first->getDebugExpression()->isEntryValue())
+    return;
----------------
dstenb wrote:
> Since `isDescribedByReg` has been changed to ignore entry values, the `InlinedEntity` will not be added to RegVars, so unless I have overlooked something I don't think we //should// get here with an entry value. Maybe this should be an assertion?
Oh, this is dead code for sure. We didn't have this in initial patch. We'll remove this. Thanks!


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

https://reviews.llvm.org/D58044