[PATCH] [DebugInfo][FastISel] Prevent using debug location from previous block for local values

[David Blaikie]

Some comments - and it'd still be good to have Alexey take a look and see if this is likely to address some of the issues in line table fidelity he knows about.

I know it won't address them all - especially when inlining within a single basic block causes the same fallthrough-like line table behavior, but at least across basic block this should help tidy things up & avoid any dependence on block ordering.


REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/SelectionDAG/FastISel.cpp:136
@@ +135,3 @@
+    // Skip past any EH_LABELs, which go before local values.
+    while (FirstLocalValue->getOpcode() == TargetOpcode::EH_LABEL)
+      ++FirstLocalValue;
----------------
I don't see anything EH related in your test cases - is this exercised/verified? (perhaps without this existing tests regress? I'm not sure)

================
Comment at: lib/CodeGen/SelectionDAG/FastISel.cpp:142
@@ +141,3 @@
+  MachineBasicBlock::iterator InstWithLoc = FuncInfo.InsertPt;
+  while (InstWithLoc != FuncInfo.MBB->end() && !InstWithLoc->getDebugLoc())
+    ++InstWithLoc;
----------------
Could use std::find_if here, if you like.

================
Comment at: test/DebugInfo/X86/dbg-line-fast-isel.ll:10
@@ +9,3 @@
+; {
+;   callme(100);
+;   if (arg > 5000)
----------------
Do the three blocks in this function all test interesting parts of the patch? Or could we use two? If they're all interesting it might be worth a comment to explain how/why.

Do these test functions need non-void returns, or could they be simplified to return void?

(does the callme function need a non-void return too? Does it need a parameter? (I assume it needs the parameter for some reason))

http://reviews.llvm.org/D9887

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/