[PATCH] D11933: Extending debug ranges

[Vikram TV via llvm-commits]

tvvikram marked 2 inline comments as done.
tvvikram added a comment.

In http://reviews.llvm.org/D11933#292339, @aprantl wrote:

> I just manually verified that this indeed works as advertised on a couple of old bugreports with debug info + ASAN. Totally awesome!


Good to hear that!

> Couple more inline comments and I think the only major thing left to do here is to agree on a name for the new pass :-)


I am okay with any of the names that you had suggested before. Please confirm. Also I will submit the current changes along with pass renaming.

> It worries me a bit that none of the test cases exhibit the case with multiple predecessors that the join() function is implementing (nested ifs without else, switch stmts...). It would be good to add one.


I have come up with a test case that does join() with 3 predecessors. Will submit along with pass renaming.


================
Comment at: lib/CodeGen/ExtendDebugRangeLocation.cpp:182
@@ +181,3 @@
+  return (Var == V.Var) && (isDescribedByReg(*MI) == isDescribedByReg(*V.MI)) &&
+         (areOffsetsEqual(*MI, *V.MI));
+}
----------------
aprantl wrote:
> What if MI is, e.g., constant 1 and V.MI is constant 2? Looks like the function would still return true because isDescribedByReg will return 0 of both. We should also compare the kind (const/reg/...) here.
> 
> Regression/Unit test that catches these corner cases?
As the TODO in line 203 says, I am handling DBG_VALUEs that describes a register. OpenRanges when built are guaranteed to describe a register.

================
Comment at: lib/CodeGen/ExtendDebugRangeLocation.cpp:339
@@ +338,3 @@
+      VarLoc V(ILT.Var, MI);
+      ILL.push_back(std::move(V));
+    }
----------------
aprantl wrote:
> Why is it necessary to push it into InLocs here? Wouldn't transfer pick it up automatically from the DBG_VALUE that we just inserted?
InLocs also represent the list of already inserted DBG_VALUEs; because we do not want to insert redundant DBG_VALUEs. Only new incoming locs that are calculated during each join() call for the MBB are inserted.


http://reviews.llvm.org/D11933