[PATCH] Change representation of instruction ranges where variable is accessible.

[David Blaikie]

================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp:117
@@ -116,3 @@
-                     const MachineInstr &MI) {
-  if (!VarHistory.empty()) {
-     const MachineInstr &Prev = *VarHistory.back();
----------------
Sorry, but I forget/don't understand why/how this code was no longer necessary.

Could you explain why it was necessary before your patch and how your patch makes it unnecessary?

Hmm, I think I see - it moved into startInstrRange, did it?

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1190
@@ +1189,3 @@
+
+      const MCSymbol *EndLabel = nullptr;
+      if (End != nullptr) {
----------------
You can drop this initialization, since the 3 cases below ensure the variable is initialized (this way, msan can catch bugs if that is ever violated)

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1191
@@ +1190,3 @@
+      const MCSymbol *EndLabel = nullptr;
+      if (End != nullptr) {
+        EndLabel = getLabelAfterInsn(End);
----------------
This if/else if chain could be done without braces, if you like (seems to be the way of things in the LLVM codebase).

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1368
@@ -1381,2 +1367,3 @@
+  // Find the end of the prologue.
   for (const auto &MBB : *MF) {
     for (const auto &MI : MBB) {
----------------
I'd probably drop the braces from all these - might be worth refactoring into a simple utility function to make this larger function more readable/simpler.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1385
@@ -1384,3 @@
-        assert(MI.getNumOperands() > 1 && "Invalid machine instruction!");
-        // Keep track of user variables in order of appearance. Create the
-        // empty history for each variable so that the order of keys in
----------------
Did the need for this ahead-of-time initialization of the MapVector go away in your change? How?

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1396
@@ +1395,3 @@
+    for (const auto &Range : Ranges) {
+      requestLabelBeforeInsn(Range.first);
+      if (Range.second)
----------------
I assume the case where the start of a range is non-null and the end of the range is null occurs for whole-function ranges? (we don't record the termination)

In that case, would it make sense to just record the location as an unbounded range (null for both start/end? No range? (I guess that'd be ambiguous with "variable is totally optimized away", though))

http://reviews.llvm.org/D3861