This is an archive of the discontinued LLVM Phabricator instance.

[BasicBlockUtils] Set debug locations for instructions created in SplitBlockPredecessors.
ClosedPublic

Authored by samsonov on Jun 9 2015, 12:21 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

samsonov updated this revision to Diff 27388.Jun 9 2015, 12:21 PM
samsonov retitled this revision from to [BasicBlockUtils] Set debug locations for instructions created in SplitBlockPredecessors..
samsonov updated this object.
samsonov edited the test plan for this revision. (Show Details)
samsonov added reviewers: eugenis, dblaikie.
samsonov added a subscriber: Unknown Object (MLST).
dblaikie edited edge metadata.Jun 9 2015, 12:51 PM

Your test only tests 2 branch instructions, yet the code change touches 3 different sites - is one of those changes untested?

samsonov updated this revision to Diff 27398.Jun 9 2015, 1:42 PM
samsonov edited edge metadata.
  • Extend test to splitting landingpad blocks.

Your test only tests 2 branch instructions, yet the code change touches 3 different sites - is one of those changes untested?

Right. Extended the test to cover all the cases.

dblaikie accepted this revision.Jun 9 2015, 1:45 PM
dblaikie edited edge metadata.

Looks good to me - thanks!

(if the test case is derived from some C/C++ source, you could include that source (& the command line - optimization flags, etc) used to produce it in a comment in the source - though I realize this sort of test just needs debug locations so it's more managable by hand now that we have legible debug info metadata)

This revision is now accepted and ready to land.Jun 9 2015, 1:45 PM
This revision was automatically updated to reflect the committed changes.

Thanks for the review! The test case is loosely derived from C++ source, but heavily minimized and changed after that, so I'd probably just leave it as is.