This is an archive of the discontinued LLVM Phabricator instance.

Summary: Fix out-of-order stepping behavior in programs with sunk instructions
ClosedPublic

Authored by ormris on Nov 10 2017, 5:09 PM.

Details

Summary

MachineSink attempts to place instructions near the basic blocks where
they are needed.

Once an instruction has been sunk, its location relative to other
instructions no longer is consistent with the original source code. In
order to ensure correct stepping in the debugger, the debug location for
sunk instructions is either merged with the insertion point or erased if the
target successor block is empty.

Event Timeline

ormris created this revision.Nov 10 2017, 5:09 PM

Friendly ping.

probinson accepted this revision.Nov 20 2017, 9:56 AM

LGTM. I'll commit this for you, as you don't have commit access yet.

This revision is now accepted and ready to land.Nov 20 2017, 9:56 AM
This revision was automatically updated to reflect the committed changes.
echristo added inline comments.Nov 20 2017, 2:05 PM
llvm/trunk/lib/CodeGen/MachineSink.cpp
874 ↗(On Diff #123621)

We don't put braces around single statement if conditions. Should probably also update the comments (in a follow-up comment) for the debug value moving to be clear what's going on there versus the location.

ormris added inline comments.Nov 21 2017, 9:09 AM
llvm/trunk/lib/CodeGen/MachineSink.cpp
874 ↗(On Diff #123621)

Will do.

ormris reopened this revision.Nov 30 2017, 2:06 PM
This revision is now accepted and ready to land.Nov 30 2017, 2:06 PM
ormris updated this revision to Diff 125012.Nov 30 2017, 2:07 PM

Change list:

  • Ensure that we don't crash when the insert position is not found.
  • Add comment explaining the debug value move.
  • Add test that tests the merging of debug values.
  • Update the previous test to use MIR.
ormris requested review of this revision.Nov 30 2017, 2:08 PM
ormris edited edge metadata.
aprantl added inline comments.Nov 30 2017, 2:09 PM
lib/CodeGen/MachineSink.cpp
885

adjacent

ormris updated this revision to Diff 125015.Nov 30 2017, 2:13 PM

Fix comment spelling.

ormris added a comment.Dec 7 2017, 9:25 AM

Friendly ping.

probinson accepted this revision.Dec 8 2017, 2:07 PM

LGTM again.

This revision is now accepted and ready to land.Dec 8 2017, 2:07 PM
ormris added a comment.Dec 8 2017, 2:40 PM

Thanks, @probinson! If someone could commit this, that would be great. I don't have commit access currently.

This revision was automatically updated to reflect the committed changes.