This is an archive of the discontinued LLVM Phabricator instance.

Propagate DBG_VALUE entries when there are unvisited predecessors
ClosedPublic

Authored by keith.walker.arm on Sep 26 2016, 10:00 AM.

Details

Summary

Variables are sometimes missing their debug location information in
blocks in which the variables should be available. This would occur
when one or more predecessor blocks had not yet been visited by the
routine which propagated the information from predecessor blocks.

This is addressed by only considering predecessor blocks which have
already been visited.

The solution to this problem was suggested by Daniel Berlin on the
LLVM developer mailing list.

Diff Detail

Repository
rL LLVM

Event Timeline

keith.walker.arm retitled this revision from to Propagate DBG_VALUE entries when there are unvisited predecessors .
keith.walker.arm updated this object.
dberlin edited edge metadata.Sep 26 2016, 10:03 AM

LGTM algorithmically, i'll let Adrian give the final okay here.

aprantl accepted this revision.Sep 26 2016, 10:15 AM
aprantl edited edge metadata.

Generally looks good to me.
The testcase appears to be a bit fragile — it might make sense to convert this to a MIR testcase so we aren't depending on random codegen choices.

lib/CodeGen/LiveDebugValues.cpp
384 ↗(On Diff #72507)

reverse

test/CodeGen/ARM/dbg-range-extension.ll
3 ↗(On Diff #72507)

propagated

This revision is now accepted and ready to land.Sep 26 2016, 10:15 AM
probinson added inline comments.
lib/CodeGen/LiveDebugValues.cpp
385 ↗(On Diff #72507)

considered

keith.walker.arm edited edge metadata.

Correct spelling mistakes in comments.
Converted the test to a MIR testcase

Thanks! Please watch the compile-time performance bots after committing this. This could cause many more DBG_VALUE instructions to be inserted and could cause a regression. Contrary to the old LiveDebugVariables pass we currently don't look at the lexical scopes before inserting a DBG_VALUE intrinsic which means that we often propagate them much further down than necessary. This is especially noticeable in large C++ functions with many inlined method calls that all use the same "this"-pointer. I mean to fix this soon but haven't gotten around to it yet.

This revision was automatically updated to reflect the committed changes.