Page MenuHomePhabricator

[DebugInfo] Keep parameter DBG_VALUEs before prologue code
ClosedPublic

Authored by dstenb on Thu, Jan 31, 6:47 AM.

Details

Summary

This is a preparatory change for removing the code from
DebugHandlerBase::beginFunction() which changes the starting label for
the first non-overlapping DBG_VALUEs of parameters to the beginning of
the function. It does that to be able to show parameters when entering a
function. However, that code does not consider what defines the values,
which can result in the ranges for the debug values starting before
their defining instructions. That code is removed in a follow-up patch.

When prologue code is inserted, it leads to DBG_VALUEs that start
directly in the entry block being moved down after the prologue
instructions. This patch fixes that by stashing away DBG_VALUEs for
parameters before emitting the prologue, and then reinserts them at the
start of the block. This assumes that there is no target that somehow
clobbers parameter registers in the frame setup; there is no such case
in the lit tests at least.

See PR40188 for more information.

Diff Detail

Repository
rL LLVM

Event Timeline

dstenb created this revision.Thu, Jan 31, 6:47 AM

LGTM, although I'll wait for someone else to approve.

bjope added a subscriber: bjope.Thu, Jan 31, 10:19 AM
bjope added inline comments.Thu, Jan 31, 10:31 AM
test/DebugInfo/X86/prolog-params.mir
1 ↗(On Diff #184494)

Add -mtriple=x86_64-unknown-linux-gnu to the RUN line.

dstenb updated this revision to Diff 185264.Tue, Feb 5, 2:57 AM

Add -mtriple to RUN line.

dstenb marked an inline comment as done.Tue, Feb 5, 2:57 AM
aprantl accepted this revision.Tue, Feb 5, 9:29 AM
aprantl added inline comments.
lib/CodeGen/PrologEpilogInserter.cpp
174 ↗(On Diff #185264)

Can you add a sentence explaining what stashing is for, something like "... the stashed DBG_VALUEs will be processed later by ..."?

183 ↗(On Diff #185264)

What kind of DebugInstr in MIR are not also a DebugValue?
Is this condition equivalent to

assert(MI.isDebugValue()) ?

184 ↗(On Diff #185264)

Nevermind. The new labels probably.

This revision is now accepted and ready to land.Tue, Feb 5, 9:29 AM
dstenb updated this revision to Diff 185525.Wed, Feb 6, 4:24 AM
dstenb marked an inline comment as done.

Amend comment for stashEntryDbgValues().

dstenb added inline comments.Wed, Feb 6, 4:26 AM
lib/CodeGen/PrologEpilogInserter.cpp
184 ↗(On Diff #185264)

Yes, this is for labels. In the cases I have looked at, the DBG_LABEL instructions are placed after the parameter DBG_VALUE instructions, but I guess there might cases where they are intermixed, or it could change in the future so that the labels are placed before.

Thanks for the reviews! I'm intending on landing this tomorrow.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptTue, Feb 12, 2:51 AM
Herald added a subscriber: jdoerfert. · View Herald Transcript