Page MenuHomePhabricator

[DebugInfo] Salvage debug info when sinking loop invariant instructions
ClosedPublic

Authored by Orlando on Apr 2 2020, 9:25 AM.

Diff Detail

Event Timeline

Orlando created this revision.Apr 2 2020, 9:25 AM
vsk accepted this revision.Apr 2 2020, 10:08 AM

Thanks, lgtm.

This revision is now accepted and ready to land.Apr 2 2020, 10:08 AM
vsk added inline comments.Apr 2 2020, 2:05 PM
llvm/lib/Transforms/Scalar/LICM.cpp
484

I took a quick look at the rest of the uses of eraseInstruction, it seems like this is the only other place where a salvage makes sense. As a follow up we might convert this to the 'OrMarkUndef' variant (or fold that into this patch).

aprantl accepted this revision.Apr 2 2020, 5:38 PM
aprantl added inline comments.
llvm/test/DebugInfo/X86/licm-undef-dbg-value.ll
84

do we need the TBAA data?

djtodoro accepted this revision.Apr 2 2020, 11:41 PM
djtodoro added inline comments.
llvm/lib/Transforms/Scalar/LICM.cpp
484

What about other uses of the salvageDebugInfo(), such as:

llvm/lib/Transforms/Scalar/GVN.cpp
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
llvm/lib/Transforms/Scalar/DCE.cpp

should we address them as well? If so, that should be a different patch.

Orlando marked 2 inline comments as done.Apr 3 2020, 12:49 AM
Orlando added a subscriber: chrisjackson.

Thanks everyone, I'll land this shortly.

llvm/lib/Transforms/Scalar/LICM.cpp
484

I believe @chrisjackson is going to fix up these cases (salvageDebugInfo -> salvageDebugInfoOrMarkUndef) in an upcoming patch.

llvm/test/DebugInfo/X86/licm-undef-dbg-value.ll
84

Nope, I'll remove it before committing.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 3 2020, 1:34 AM