This is an archive of the discontinued LLVM Phabricator instance.

[EarlyCSE] Salvage debug info during DCE
ClosedPublic

Authored by djtodoro on Dec 21 2017, 8:46 AM.

Details

Summary

EarlyCSE didn't try to salvage debug info during erasing of instructions. This fixes that.

Diff Detail

Repository
rL LLVM

Event Timeline

djtodoro created this revision.Dec 21 2017, 8:46 AM
vsk added a subscriber: vsk.Dec 21 2017, 10:48 AM
davide added a subscriber: davide.Dec 23 2017, 5:34 AM
vsk added a comment.Jan 5 2018, 3:20 PM

I think the test case could probably be reduced a bit (is 'ctpop' necessary?), but apart from that, this looks good. @aprantl wdyt?

davide requested changes to this revision.Jan 5 2018, 3:27 PM
davide added inline comments.
test/Transforms/EarlyCSE/debuginfo-dce.ll
1–7 ↗(On Diff #127893)

I'm under the impression this test case can be simplified. Can you give it a try?

This revision now requires changes to proceed.Jan 5 2018, 3:27 PM

LGTM otherwise.

djtodoro updated this revision to Diff 128928.Jan 8 2018, 7:38 AM

@vsk I have updated it. 'ctpop' is not necessary, there can be other intrinsic (e.g. 'bswap'). Thanks for the comment!
@aprantl Thanks!

test/Transforms/EarlyCSE/debuginfo-dce.ll
1–7 ↗(On Diff #127893)

Yes, I have updated it. Thank you for the comment!

davide accepted this revision.Jan 8 2018, 7:50 AM

LGTM modulo minor.

test/Transforms/EarlyCSE/debuginfo-dce.ll
2–3 ↗(On Diff #128928)

unneeded.

This revision is now accepted and ready to land.Jan 8 2018, 7:50 AM
davide added a comment.Jan 8 2018, 7:51 AM

@djtodoro can you commit this or you need somebody to do it on your behalf?

djtodoro updated this revision to Diff 128930.Jan 8 2018, 7:59 AM
djtodoro marked an inline comment as done.

@davide one of my colleagues will commit it

This revision was automatically updated to reflect the committed changes.