[PATCH] Rewrite calculateDbgValueHistory to make it (hopefully) more transparent.

[David Blaikie]

================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp:37
@@ +36,3 @@
+  // the location of variable is unknown.
+  if (!MI.getOperand(0).isReg() || MI.getOperand(0).getReg() == 0U)
+    return None;
----------------
Alexey Samsonov wrote:
> David Blaikie wrote:
> > Is the first operand ever not a register? I thought that was an invariant of these intrinsics.
> > 
> > So it looks like isDescribedByReg boils down to "return MI.getOperand(0).getReg()", and some assertions?
> > 
> > (assuming the return type changes to "unsigned" rather than "Optional<unsigned>" - correctly returning 0/false when there is no register use (ie: this is a range terminator))
> > 
> > 
> No. See getDebugLocValue() in DwarfDebug.cpp which actually distinguishes between different kinds of MachineOperands used as a first argument of DBG_VALUE. I've updated the comment in this function.
OK. I see, it's either a register or a constant.

I'd still probably switch to just using an unsigned return value, dropping the Optional. Since zero is "not in a register" anyway.

  if (!MI.getOperand(0).isReg())
    return 0;
  return MI.getOperand(0).getReg();

I know it's a bit more subtle, using 0 as the "not in a register" state, but having two null states (Optional-with-value-zero and Optional-without-value) isn't ideal either... 

================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp:164
@@ +163,3 @@
+
+    // Make sure locations for register-described variables are valid only
+    // until the end of the basic block.
----------------
This is the normal for-loop exit condition, right?

Rather than breaking early (I found this hard to parse because I wasn't sure if it was exiting the loop early under some interesting condition - having to think about the addresses of elements, etc, was a bit taxing) could we just roll the condition into the clobberAllRegistersUses instead?

I suppose the condition will still be hard to pass, but I won't be trying to think about non-linear control flow at the same time.

Maybe the comment could be clarified here. "<existing comment> unless it's the last basic block, in which case let their liveness run off to the end of the function" or something?
  if (!MBB.empty() && &MBB != &MF->back())

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1271
@@ -1270,3 +1270,3 @@
                      << "\t" << *Begin << "\t" << *End << "\n");
-        if (End->isDebugValue())
+        if (End->isDebugValue() && End->getDebugVariable() == DV)
           SLabel = getLabelBeforeInsn(End);
----------------
What's the reason for these extra conditions? (the getDebugVariable() == DV checks here and below)

http://reviews.llvm.org/D3597