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

code_* cleanup, and strip IR metadata from code_llvm output #10747

Merged
merged 5 commits into from
Apr 7, 2015

Conversation

ihnorton
Copy link
Member

@ihnorton ihnorton commented Apr 5, 2015

(replaces #10741 ... because github)

The first commit cleans up the code_native and code_llvm pathways a little bit by removing an intermediate function and calling the one we want directly from Julia code.

The second commit changes the behavior of code_llvm to strip all metadata before printing the IR. This reduces the work needed to run code_llvm output with lli for debugging purposes. I also added a code_llvm_raw version with the current behavior. If people don't think stripped should be the default I'll switch it around.

Should now compile against gcc 4.6.

before:

; Function Attrs: uwtable
define void @julia_bf_112785(i32, i64) #0 {
top:
  call void @llvm.dbg.value(metadata i32 %0, i64 0, metadata !11, metadata !17)
  call void @llvm.dbg.value(metadata i64 %1, i64 0, metadata !18, metadata !17)
  %sext = shl i32 %0, 24, !dbg !19
  %2 = ashr exact i32 %sext, 24, !dbg !19
  %3 = icmp eq i32 %2, %0, !dbg !19
  br i1 %3, label %pass, label %fail, !dbg !19
...

after:

; Function Attrs: uwtable
define void @julia_bf_112785(i32, i64) #-1 {
top:
  %sext = shl i32 %0, 24
  %2 = ashr exact i32 %sext, 24
  %3 = icmp eq i32 %2, %0
  br i1 %3, label %pass, label %fail

@ihnorton ihnorton force-pushed the dumper_cleanup2 branch 2 times, most recently from 47f55f0 to 17e8b2a Compare April 6, 2015 03:27
@ihnorton
Copy link
Member Author

ihnorton commented Apr 6, 2015

Now removes these lines as well:

  call void @llvm.dbg.value(metadata i32 %0, i64 0, metadata !0, metadata !13)
  call void @llvm.dbg.value(metadata i64 %1, i64 0, metadata !14, metadata !13)

Also added a helper function jl_function_ptr_by_llvm_name. The purpose is to allow copying a mangled julia_... name from a code_llvm dump, then call jl_function_ptr_by_llvm_name to get the corresponding Function*, then call the refactored dumper functions directly (without needing to figure out the corresponding Julia signature).

It would be nice to have a way to go all the way back to the original Julia method signature from one of these mangled names, but I don't think we store that mapping anywhere. FuncInfo has the Function* and the name, but to get back to the original type signature would (I think) require iterating over all methods and looking at the attached functionObject.

@Keno
Copy link
Member

Keno commented Apr 6, 2015

Thanks @ihnorton. I can't quite tell if the windows failure is related or not, but otherwise, LGTM. This is a much needed cleanup.

@tkelman
Copy link
Contributor

tkelman commented Apr 6, 2015

Definitely related. The added include from codegen.cpp:11 either needs a flag that is getting set in the linux build but not the windows build, or the headers themselves go down different codepaths which are broken for windows

@ihnorton
Copy link
Member Author

ihnorton commented Apr 6, 2015

Thanks. Should be fixed now, will let appveyor run through. I am on windows, but using llvm-3.6 and forgot to remove a line that is incompatible with 3.3.

@ihnorton
Copy link
Member Author

ihnorton commented Apr 6, 2015

Also added this notice to the output of code_llvm:

; NOTE: Instruction metadata and dbg.* calls have been removed.
;       Use Base.code_llvm_raw for full IR.

@StefanKarpinski
Copy link
Member

I don't think the notice is necessary – it should suffice to put this in the documentation of code_llvm.

@simonster
Copy link
Member

This certainly shouldn't hold up this PR, but is it possible to turn the debug metadata into line numbers? Right now we get them in code_typed and code_native, but not code_llvm.

@ihnorton
Copy link
Member Author

ihnorton commented Apr 6, 2015

Good point -- that would be a much more useful thing to do with the data before throwing it away. I'll have to get a bit more familiar with how the debug locations work, but will see what I can do (there is enough information in the system to eventually make the DWARF annotations, so it must be possible).

Remove intermediate function, now pick the action and call directly from the Julia side
with a pointer to the LLVM::Function.
This makes IR output easier to use with external tools.
Useful with the _ir and _asm dump functions to trace through a call path
without figuring out all of the necessary Julia signatures.
ihnorton added a commit that referenced this pull request Apr 7, 2015
code_* cleanup, and strip IR metadata from code_llvm output
@ihnorton ihnorton merged commit 2459f7f into JuliaLang:master Apr 7, 2015
@ihnorton ihnorton deleted the dumper_cleanup2 branch April 7, 2015 03:53
@pao pao mentioned this pull request Apr 7, 2015
@simonster
Copy link
Member

@ihnorton Is it possible to show just the TBAA metadata? I'm finding that I miss it. I know I can get it with code_llvm_raw, but that's a bit less convenient since there's no @code_llvm_raw macro. Alternatively I could just add that macro.

@ihnorton
Copy link
Member Author

@simonster this will preserve TBAA MDNodes:

diff --git a/src/codegen.cpp b/src/codegen.cpp
index 4575080..b2a8f34 100644
--- a/src/codegen.cpp
+++ b/src/codegen.cpp
@@ -1016,6 +1016,8 @@ const jl_value_t *jl_dump_function_ir(void *f, bool strip_ir_metadata)

                 // iterate over all metadata kinds and set to NULL to remove
                 for (; md_iter != MDForInst.end(); ++md_iter) {
+                    if ((*md_iter).first == LLVMContext::MD_tbaa)
+                        continue;
                     inst->setMetadata((*md_iter).first, NULL);
                 }
             }

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.

5 participants