Skip to content
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

IDEA: patch LLVM to make 'code_llvm_ display source location information #19342

Closed
wants to merge 1 commit into from

Conversation

maleadt
Copy link
Member

@maleadt maleadt commented Nov 16, 2016

This is a debugging patch I had floating around, which multiple people have mentioned would be very useful to have by default. It patches LLVM's AssemblyWriter to make it emit source location information:

julia> code_llvm(STDOUT, sin, Tuple{Int}, false)

define double @julia_sin_61930(i64) #0 !dbg !5 {
top:
; Filename: math.jl
; Source line: 265
  %1 = sitofp i64 %0 to double, !dbg !7
  %2 = call double @julia_sin_61931(double %1) #0, !dbg !7
  ret double %2, !dbg !7
}

Not sure if this is worth patching LLVM for, but it has helped me greatly when hunting down the origin of runtime calls in LLVM IR output. If there's interest, I could clean this up (make sure it works on all supported LLVM versions, make sure it works with @code_llvm, maybe try to still have it strip !dbg metadata, etc).

cc @andreasnoack, @jrevels

@Keno
Copy link
Member

Keno commented Nov 16, 2016

I'm hesitant to take any patch that's not at least submitted for review upstream. As for the change itself, it seems fine to me, though I think I'd like an option to disable the verbose line number printing.

@maleadt
Copy link
Member Author

maleadt commented Nov 16, 2016

I can look into upstreaming this, but I expect that to take a while (don't have time right now, and it'll be bikeshedded to death). Some way of disabling this would be required indeed, as it breaks many FileCheck tests (not sure if we run LLVM's tests on any buildbot?).

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jul 15, 2017

I'd still like to see this upstreamed :). I've so far found that the LLVM folks are pretty happy to take good patches and not bikeshed them to death. Probably fine to just propose a command line argument / global configuration variable that sets the default value for an argument to the AssemblyWriter constructor.

@vtjnash vtjnash closed this Jul 15, 2017
@tkelman tkelman deleted the tb/asmwriter_di branch July 15, 2017 06:15
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.

3 participants