Page MenuHomePhabricator

[Local] Add dbg location on unreachable inst in changeToUnreachable

Authored by gramanas on Aug 6 2018, 10:24 AM.



As show in
it would be desirable to have debug location in the unreachable

If the unreachable is lowered to llvm.trap() or similar,
it could be really useful to have line/scope information there
to make forensic debugging more pleasant.

I am not sure whether to add a unit test for this funtion.

Diff Detail


Event Timeline

gramanas created this revision.Aug 6 2018, 10:24 AM
vsk added a comment.Aug 6 2018, 10:50 AM

Thanks for doing this! Yes, please add a unit test in unittests/Transforms/Utils/Local.cpp.

gramanas updated this revision to Diff 159561.Aug 7 2018, 11:53 AM

Add unit test.

vsk added inline comments.Aug 7 2018, 12:01 PM
616 ↗(On Diff #159561)

Why not just .front()?

620 ↗(On Diff #159561)

nit, 1U is easier to read.

620 ↗(On Diff #159561)

Please add a comment when using boolean literals as parameters. Instead of 'false', it's clearer to write /*UseLLVMTrap*/false.

626 ↗(On Diff #159561)

This needs to check that the correct location is present, not just that the instruction has metadata.

gramanas updated this revision to Diff 159572.Aug 7 2018, 12:29 PM
gramanas marked 4 inline comments as done.
  • update according to comments
vsk accepted this revision.Aug 7 2018, 12:31 PM

LGTM, thanks!

627 ↗(On Diff #159572)

Nit, this assertion is now redundant.

This revision is now accepted and ready to land.Aug 7 2018, 12:31 PM
This revision was automatically updated to reflect the committed changes.
gramanas marked an inline comment as done.