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

[Jeremy Morse via Phabricator via llvm-commits]

jmorse accepted this revision.
jmorse added a comment.

This is all fine and well; are redundant variable locations a common case and which should we optimise for? IIUC every instruction with debug-info results in a SmallVector copy right now, which isn't necessary if that specific position hasn't had debug-info change. IMO, adding a "ChangedPosition" flag that's used to skip `setWedge` if it isn't true would be beneficial.



================
Comment at: llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp:1421
+    for (auto RIt = Locs->rbegin(), REnd = Locs->rend(); RIt != REnd; ++RIt) {
+      DebugVariable Key = FnVarLocs.getVariable(RIt->VariableID);
+      bool FirstDefOfFragment = VariableSet.insert(Key).second;
----------------
Ideally `Key` would be a reference to avoid any un-necessary copying, I think doing that hinges on feedback in the parent patch.


================
Comment at: llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp:1466
+  // instead call DefineBits and HasDefinedBits.
+  DenseMap<DebugAggregate, DenseSet<DIExpression::FragmentInfo>> VarsWithDef;
+  // Specify that V (a fragment of A) has a non-undef location.
----------------
One extra malloc per block seems like quite a bit, use `SmallDenseMap`?


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

https://reviews.llvm.org/D136321