• addressing comment

• addressing comments

In D87233#2288809, @dstenb wrote:

Sorry for a piecemeal review from my part!

No problem, the tempo works for me. :) Thanks!

• Rebase

• Update the algorithm for finding the entry values
• Improve the tests

• addressing comments
• refactoring
• fixing tests
• adding RegState::Debug to the Reg from the dbg_value

@aprantl thanks for your comments.

@dstenb Please let me know if this looks OK to you now.

Please investigate the test failures.
($ make check-all)

LGTM, thanks. The motivation is D87903?

• Rebasing

• Rebasing
• Refactoring

• Addressing comments
• Refactoring
• Rename debugify passes into more general name (in order to support original debug info check) - VerifyDIPreserve
• Rename the options
• Fix the test cases
• Update the docs
• Rebasing

@aprantl @vsk Thanks a lot for the reviews!

will make right fix for this (coming soon)

• Test new expression

-Split into 2 parts
-Remove unused parts

@dstenb Thanks a lot for your comments! I will split this into two patches.

(test.c was listed within llvm/test/DebugInfo/X86/entry-values-for-isel-invalidated-nodes.ll)
Looks like LLDB has a problem with parsing "complex" expression with DW_OP_entry_value:

This is actually ISel phase, but we still consider SSA, so this should be the latest point we are able to use the findEntryValue() in the pipeline.

@dblaikie Thanks for your comment!

Ping :)

LGTM, thanks.

Please close the revision as well.

nit included, otherwise lgtm, thanks!

@Orlando Thanks for doing this!

In D85018#2194109, @dblaikie wrote:

In D85018#2193545, @SouraVX wrote:

In D85018#2193489, @djtodoro wrote:

According to the --show-section-sizes output, the .debug_loc has been increased?

Thanks for reviewing this! Yea that's one of the drawback(Added FIXME for the same).

When you say "this causes loc list emission even at -O0" could you be more precise? Does the debug_info actually use a location list somewhere? If so, where, if not - does debug_loc contain an unused location list? Does it contain no location lists, but gets emitted (though empty) when it'd otherwise not be emitted at all?

+1 for this question(s).

According to the --show-section-sizes output, the .debug_loc has been increased?

From the [0], I've understood that this should affect debug build only (-g + -O0) ?

@StephenTozer Thanks for addressing the comments! Can you please share with us if you have any measurements with the respect to build time increase (e.g. on clang build itself) with this patch? I think that this patch is the only one from the stack that could give us potential performance issues.

• additional formatting

• refactor the comments

@craig.topper Thanks for addressing the comments, lgtm now.

Hi @vsk, is this in better shape now? :)

@craig.topper Thanks for fixing this!

@vsk Thanks!

Any additional comment on this? :)

Thanks for doing this!

In D83047#2150869, @jmorse wrote:

On the topic of entry values, I believe the value based tracking should make identifying entry values trivial -- all entry value numbers will be identifiable (as a ValueIDNum) with "BlockNo=0" and "InstNo="0", indicating a value defined before the first instruction of the first block. Ideally, the logic would be after machine-value-numbers and variable-values are propagated, when final locations are picked, and would look like this:

• This variables value is an entry value (BlockNo=0,InstNo=0),
• That value isn't available in any machine location right now
• I will emit a DBG_VALUE containing a DW_OP_LLVM_entry_value expression.

My initial impression went in that direction. The implementation of the callee side of the entry values feature (production of the DW_OP_entry_values) within GCC Value Tracking System was trivial as well. I think this new implementation of LLVM LDV will give us more freedom to use entry-values even for modified parameters, if we can express the modification in terms of its entry value (i.e. param = param + 2 ==> DW_OP_plus (DW_OP_entry_value (param), 2)).

• Improve the test

• Rebase

• Rebase

• Rebase
• TODO: Consider another name for mode. 'original' might be too sharp.

• Move the def of DebugifyCustomPassManager into the Debugify.h

• Update the comment around ShouldEmitDebugEntryValues()

Thanks!