[PATCH] D101064: [WIP] improve debug-info in stack-slot-coloring

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

Looks good as a starting place -- I took this for  test drive, see inline comments. On a clang-3.4 build, the "PC ranges covered" statistic ticks down from 66% to 65%, however I believe that's due to rounding. Using llvm-locstats, roughly six thousand variables move out of the 100% covered bucket, or about 0.2% of all variables in clang-3.4. 0% covered goes up by a negligible amount. In my opinion: this is a reasonable sacrifice to make in the name of accuracy, and some of those dropped locations will be incorrect. Plus, there's a longer term plan for not needing this.

The LiveIntervals range only identifies spans of instructions: it doesn't contain any information about control flow. There can be scenarios where a stack slot goes out of liveness but doesn't get terminated by this implementation: for example, when we branch out of of a block, the branch target might be somewhere where the slot isn't live.

Seeing how this is a pragmatic fix to eliminate some false locations, I don't think we need to cover all circumstances with a patch like this -- up to you if you want to cover the extra scenarios.



================
Comment at: llvm/lib/CodeGen/StackSlotColoring.cpp:548
+      if (MI.isDebugValue() && MI.getOperand(0).isFI()) {
+        auto SS = MI.getOperand(0).getIndex();
+        SS2DbgValues[SS].push_back(&MI);
----------------
I get an assertion about SS being a tombstone value for SS2DbgValues, sometimes it comes out as -2. Elsewhere there are loops that continue/bypass if FI is negative, perhaps that's needed here?


================
Comment at: llvm/lib/CodeGen/StackSlotColoring.cpp:569
     Assignments[i].clear();
   Assignments.clear();
 
----------------
Needs to clear the collections you added to the pass


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

https://reviews.llvm.org/D101064