This is an archive of the discontinued LLVM Phabricator instance.

[DebugInfo][Dexter] Unreachable line stepped onto after SimplifyCFG.
ClosedPublic

Authored by CarlosAlbertoEnciso on Oct 15 2018, 7:12 AM.

Details

Summary

For the below test:

int main() {
  volatile int foo = 0;
  int read1 = foo;
  int brains = foo;

  if (read1 > 3) {
    brains *= 2;
    brains += 1;
  }

  return brains;
}

When in the debugger, on the line "if (read1 > 3)", and we step from the
'if' condition, onto the addition, then back to the 'if' again, which is
misleading, as described by:

PR38762

Notes:

  • The revision D52887 closed by commit rL344120 introduced a regression and now is abandoned.
  • Reverted in rL344135 as it was causing buildbot failure.
  • This patch includes a fix for the regression described by PR39243 as well.

Diff Detail

Repository
rL LLVM

Event Timeline

aprantl accepted this revision.Oct 15 2018, 9:05 AM
aprantl added inline comments.
lib/Transforms/Utils/Local.cpp
2532 ↗(On Diff #169699)

clang-format please

2549 ↗(On Diff #169699)

As I explained in https://bugs.llvm.org/show_bug.cgi?id=39141#c8 the more fundamental reason why the dbg.values need to be deleted is because there won't be any instructions with a DILocation in either branch left after performing the transformation. We can only insert a dbg.value after the two branches are joined again.

This revision is now accepted and ready to land.Oct 15 2018, 9:05 AM
CarlosAlbertoEnciso marked 2 inline comments as done.

Address issues raised by @aprantl.

@vsk: Do you have any additional comments.

Thanks to the reviewers for their comments and suggestions.

lib/Transforms/Utils/Local.cpp
2532 ↗(On Diff #169699)

Done.

2549 ↗(On Diff #169699)

I have included your comments for a better understanding of 'hoistAllInstructionsInto'.

aprantl accepted this revision.Oct 16 2018, 8:46 AM

@vsk: Do you have any additional comments? It is OK for you if I commit the patch?

Thanks

vsk accepted this revision.Oct 24 2018, 8:04 AM

LGTM, thanks. I'm sorry for the delay in reviewing this.

In D53287#1274290, @vsk wrote:

LGTM, thanks. I'm sorry for the delay in reviewing this.

No apologies needed. Thanks

@aprantl, @vsk: Thanks very much for your valuable comments and suggestions.

This revision was automatically updated to reflect the committed changes.