[PATCH] D136321: [Assignment Tracking Analysis][2/*] Remove redundant location definitions

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer accepted this revision.
StephenTozer added a comment.
This revision is now accepted and ready to land.

Minor comments, otherwise LGTM.



================
Comment at: llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp:1453
+static bool
+removeRedundantDbgLocsUsingForwardScan(const BasicBlock *BB,
+                                       FunctionVarLocsBuilder &FnVarLocs) {
----------------
YMMV so just a suggestion, but could the entry block stuff be moved into a separate function? As-is there's a DenseMap in this function, which is reasonably large even when unused (and possibly can't be optimized out by the compiler since it's dependent on the return value of `BB->isEntryBlock()`?), and in terms of behaviour it seems like two fairly distinct behaviours bundled into one function. I can also see how it may make sense to have them together in a single forward-scanning function though, so your call on whether to split it or not.


================
Comment at: llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp:1499
+
+    // Iterate pver the existing defs.
+    for (const VarLocInfo &Loc : *Locs) {
----------------



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

https://reviews.llvm.org/D136321