Page MenuHomePhabricator

Don't break bundles when adding DBG_VALUE
Needs RevisionPublic

Authored by npjdesres on Jul 14 2017, 1:19 PM.

Details

Summary

LiveDebugValues should test whether an instruction is within a bundle before inserting a DBG_VALUE instruction.

Diff Detail

Event Timeline

npjdesres created this revision.Jul 14 2017, 1:19 PM
davide requested changes to this revision.Jul 14 2017, 1:22 PM
davide added a subscriber: davide.

This change has no test associated. Can you please add one?
Also, this patch is oddly formatted, can you please run clang-format on it?

This revision now requires changes to proceed.Jul 14 2017, 1:22 PM
probinson added inline comments.
lib/CodeGen/LiveDebugValues.cpp
671

Please add a FIXME to preserve these DBG_VALUEs. I don't know how to do that, but losing them is not great.

wolfgangp added inline comments.Jul 14 2017, 4:02 PM
lib/CodeGen/LiveDebugValues.cpp
671

I agree with Paul here. The DBG_VALUE instruction should be inserted after the bundle then.

Thank you so far, and sorry for the radio silence. I'll work on a testcase and revising this patch, but things just picked up in my office, so it may be next week.

bjope added a subscriber: bjope.Sep 5 2017, 1:40 PM
materi added a subscriber: materi.Sep 12 2017, 1:35 AM
materi added inline comments.Sep 12 2017, 1:53 AM
lib/CodeGen/LiveDebugValues.cpp
671

But we must not put the DBG_VALUE after the bundle if the bundle also modifies the frame pointer. We recently had such a bug in my out-of-tree back-end.

I think moving a DBG_VALUE will require some analysis to make sure the live range is not moved.