[PATCH] D52887: [DebugInfo][Dexter] Unreachable line stepped onto after SimplifyCFG.

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso marked 2 inline comments as done.
CarlosAlbertoEnciso added inline comments.


================
Comment at: lib/Transforms/Utils/Local.cpp:2535
+  // Since we are moving the instructions out of its basic block, we do not
+  // retain their original debug location and debug intrinsic instructions.
+  // Doing so would degrade the debugging experience and adversely affect the
----------------
aprantl wrote:
> CarlosAlbertoEnciso wrote:
> > CarlosAlbertoEnciso wrote:
> > > vsk wrote:
> > > > aprantl wrote:
> > > > > Please also explain why if possible. I think the reason is different for DILoctions and dbg.values.
> > > > Right. Moving locations around degrades single-stepping and profiling. Moving dbg.values around makes variable updates appear re-ordered in a debug session.
> > > Updated the comments to include a better explanation about the actions taken on DILocations and dbg.values.
> > Taking your feedback, as the base for the updated comments.
> Just to be clear: When I said "please explain" I meant: in the comment in the code, not to me in the review :-)
I have added extra comments in the code for the function 'hoistAllInstructionsInto'.


================
Comment at: test/CodeGen/X86/pr38762.ll:51
+  %foo.0..sroa_cast = bitcast i32* %foo to i8*
+  store volatile i32 0, i32* %foo, align 4, !tbaa !18
+  %foo.0. = load volatile i32, i32* %foo, align 4, !tbaa !18
----------------
aprantl wrote:
> Do we need all the !tbaa metadata for the test?
Good point. I have removed all the !tbaa metadata. It is not required.


https://reviews.llvm.org/D52887