Skip to content

Improve DebugInfo source provenance #54

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

serenity4
Copy link
Member

@serenity4 serenity4 commented Jun 6, 2025

This PR accomplishes better DebugInfo tracing via the following changes:

  • The insertion of DebugInfo was changed to be on top of other DebugInfo nodes. This notably allows Add codegen option to associate debuginfo with statement number at a particular stage #39 to retain source information.
  • @insert_node_here uses its __source__ implicit macro argument to insert additional DebugInfo information, if the insert_stmt_debuginfo setting is enabled.
  • Similarly, replace_call! now takes a source and a settings struct to conditionally insert additional DebugInfo.
  • Most insert_node_here! calls were converted to the macro form, @insert_node_here. Those that remain in function form use additional source and settings arguments to indicate provenance for optional DebugInfo.
  • slotnames information has been added to the ODE/DAE RHS codegen, and to the factory IR.

I also included a refactor for the naming of locals in the ODE codegen while I was at it; see 676875a. I find it clearer, but if it's just me I am open to reverting that.

To do:

  • See if we can turn the multiple "macro expansion" annotations into something more readable.
  • Address the crash observed in test/ipo.jl.

@serenity4
Copy link
Member Author

For reference, this is what we now get:
screenshot

Looking into the chained macro expansion lines, what we'd need to have them show in a nicer way (i.e., nesting properly instead of accumulating on the same line) is to extract a method (any method) of the function updating/inserting a statement. For example, statement %2 in the screenshot is traced to tearing/schedule.jl:49, which is ir_mul_const!, so we'd need something like first(methods(ir_mul_const!)) and use that in debuginfo.def. This is what proper nesting would look like (don't mind the obviously wrong method/lines):

screenshot_2

as opposed to e.g. %4 in the first screenshot where certain sources accumulate on the same line.

However, I don't think we have a non-intrusive way to do this, so I don't think that'd be worth it in the end and the current state seems largely good enough.

Comment on lines +75 to +83
# Work around `show` functions requiring `invokelatest` queries
# that may be problematic to execute from within generated functions.
local inst
try
inst = string(stmt[:inst])
catch e
isa(e, UndefVarError) && continue
rethrow()
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a bit of a hack, though calling show is kind of UB from generated functions. I'm only allowing UndefVarError here which are currently caused by binding lookups in certain cases, but it's not impossible that we later discover that other exceptions may be thrown.

@serenity4 serenity4 marked this pull request as ready for review June 9, 2025 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant