Address review comments, plus some minor refactoring.

maleadt · maleadt · commit 474b394c60a3 · 2023-05-26T11:39:12.000+02:00
diff --git a/src/codegen-stubs.c b/src/codegen-stubs.c
@@ -18,7 +18,7 @@ JL_DLLEXPORT void jl_get_llvm_external_fns_fallback(void *native_code, arraylist
 
 JL_DLLEXPORT void jl_extern_c_fallback(jl_function_t *f, jl_value_t *rt, jl_value_t *argt, char *name) UNAVAILABLE
 JL_DLLEXPORT jl_value_t *jl_dump_method_asm_fallback(jl_method_instance_t *linfo, size_t world,
-        char raw_mc, char getwrapper, const char* asm_variant, const char *debuginfo, char binary) UNAVAILABLE
+        char emit_mc, char getwrapper, const char* asm_variant, const char *debuginfo, char binary) UNAVAILABLE
 JL_DLLEXPORT jl_value_t *jl_dump_function_ir_fallback(jl_llvmf_dump_t *dump, char strip_ir_metadata, char dump_module, const char *debuginfo) UNAVAILABLE
 JL_DLLEXPORT void jl_get_llvmf_defn_fallback(jl_llvmf_dump_t *dump, jl_method_instance_t *linfo, size_t world, char getwrapper, char optimize, const jl_cgparams_t params) UNAVAILABLE
 
@@ -83,9 +83,9 @@ JL_DLLEXPORT void jl_dump_llvm_opt_fallback(void *s)
 {
 }
 
-JL_DLLEXPORT jl_value_t *jl_dump_fptr_asm_fallback(uint64_t fptr, char raw_mc, const char* asm_variant, const char *debuginfo, char binary) UNAVAILABLE
+JL_DLLEXPORT jl_value_t *jl_dump_fptr_asm_fallback(uint64_t fptr, char emit_mc, const char* asm_variant, const char *debuginfo, char binary) UNAVAILABLE
 
-JL_DLLEXPORT jl_value_t *jl_dump_function_asm_fallback(jl_llvmf_dump_t* dump, char raw_mc, const char* asm_variant, const char *debuginfo, char binary) UNAVAILABLE
+JL_DLLEXPORT jl_value_t *jl_dump_function_asm_fallback(jl_llvmf_dump_t* dump, char emit_mc, const char* asm_variant, const char *debuginfo, char binary, char raw) UNAVAILABLE
 
 JL_DLLEXPORT void jl_get_function_id_fallback(void *native_code, jl_code_instance_t *ncode,
         int32_t *func_idx, int32_t *specfunc_idx) UNAVAILABLE
diff --git a/src/disasm.cpp b/src/disasm.cpp
@@ -575,7 +575,7 @@ static uint64_t compute_obj_symsize(object::SectionRef Section, uint64_t offset)
 
 // print a native disassembly for the function starting at fptr
 extern "C" JL_DLLEXPORT_CODEGEN
-jl_value_t *jl_dump_fptr_asm_impl(uint64_t fptr, char raw_mc, const char* asm_variant, const char *debuginfo, char binary)
+jl_value_t *jl_dump_fptr_asm_impl(uint64_t fptr, char emit_mc, const char* asm_variant, const char *debuginfo, char binary)
 {
     assert(fptr != 0);
     std::string code;
@@ -600,7 +600,7 @@ jl_value_t *jl_dump_fptr_asm_impl(uint64_t fptr, char raw_mc, const char* asm_va
         return jl_pchar_to_string("", 0);
     }
 
-    if (raw_mc) {
+    if (emit_mc) {
         return (jl_value_t*)jl_pchar_to_array((char*)fptr, symsize);
     }
 
@@ -1203,7 +1203,7 @@ class LineNumberPrinterHandler : public AsmPrinterHandler {
 
 // get a native assembly for llvm::Function
 extern "C" JL_DLLEXPORT_CODEGEN
-jl_value_t *jl_dump_function_asm_impl(jl_llvmf_dump_t* dump, char raw_mc, const char* asm_variant, const char *debuginfo, char binary)
+jl_value_t *jl_dump_function_asm_impl(jl_llvmf_dump_t* dump, char emit_mc, const char* asm_variant, const char *debuginfo, char binary, char raw)
 {
     // precise printing via IR assembler
     SmallVector<char, 4096> ObjBufferSV;
@@ -1218,13 +1218,14 @@ jl_value_t *jl_dump_function_asm_impl(jl_llvmf_dump_t* dump, char raw_mc, const
                     f2.deleteBody();
             }
             // add a nounwind attribute to get rid of cfi instructions
-            f->addFnAttr(Attribute::NoUnwind);
+            if (!raw)
+                f->addFnAttr(Attribute::NoUnwind);
         });
         auto TMBase = jl_ExecutionEngine->cloneTargetMachine();
         LLVMTargetMachine *TM = static_cast<LLVMTargetMachine*>(TMBase.get());
         legacy::PassManager PM;
         addTargetPasses(&PM, TM->getTargetTriple(), TM->getTargetIRAnalysis());
-        if (raw_mc) {
+        if (emit_mc) {
             raw_svector_ostream obj_OS(ObjBufferSV);
             if (TM->addPassesToEmitFile(PM, obj_OS, nullptr, CGFT_ObjectFile, false, nullptr))
                 return jl_an_empty_string;
diff --git a/src/jitlayers.cpp b/src/jitlayers.cpp
@@ -580,14 +580,14 @@ void jl_generate_fptr_for_unspecialized_impl(jl_code_instance_t *unspec)
 // get a native disassembly for a compiled method
 extern "C" JL_DLLEXPORT_CODEGEN
 jl_value_t *jl_dump_method_asm_impl(jl_method_instance_t *mi, size_t world,
-        char raw_mc, char getwrapper, const char* asm_variant, const char *debuginfo, char binary)
+        char emit_mc, char getwrapper, const char* asm_variant, const char *debuginfo, char binary)
 {
     // printing via disassembly
     jl_code_instance_t *codeinst = jl_generate_fptr(mi, world);
     if (codeinst) {
         uintptr_t fptr = (uintptr_t)jl_atomic_load_acquire(&codeinst->invoke);
         if (getwrapper)
-            return jl_dump_fptr_asm(fptr, raw_mc, asm_variant, debuginfo, binary);
+            return jl_dump_fptr_asm(fptr, emit_mc, asm_variant, debuginfo, binary);
         uintptr_t specfptr = (uintptr_t)jl_atomic_load_relaxed(&codeinst->specptr.fptr);
         if (fptr == (uintptr_t)jl_fptr_const_return_addr && specfptr == 0) {
             // normally we prevent native code from being generated for these functions,
@@ -635,15 +635,15 @@ jl_value_t *jl_dump_method_asm_impl(jl_method_instance_t *mi, size_t world,
             }
         }
         if (specfptr != 0)
-            return jl_dump_fptr_asm(specfptr, raw_mc, asm_variant, debuginfo, binary);
+            return jl_dump_fptr_asm(specfptr, emit_mc, asm_variant, debuginfo, binary);
     }
 
     // whatever, that didn't work - use the assembler output instead
     jl_llvmf_dump_t llvmf_dump;
     jl_get_llvmf_defn(&llvmf_dump, mi, world, getwrapper, true, jl_default_cgparams);
     if (!llvmf_dump.F)
         return jl_an_empty_string;
-    return jl_dump_function_asm(&llvmf_dump, raw_mc, asm_variant, debuginfo, binary);
+    return jl_dump_function_asm(&llvmf_dump, emit_mc, asm_variant, debuginfo, binary, false);
 }
 
 CodeGenOpt::Level CodeGenOptLevelFor(int optlevel)
diff --git a/src/julia_internal.h b/src/julia_internal.h
@@ -1658,11 +1658,11 @@ typedef struct {
 } jl_llvmf_dump_t;
 
 JL_DLLIMPORT jl_value_t *jl_dump_method_asm(jl_method_instance_t *linfo, size_t world,
-        char raw_mc, char getwrapper, const char* asm_variant, const char *debuginfo, char binary);
+        char emit_mc, char getwrapper, const char* asm_variant, const char *debuginfo, char binary);
 JL_DLLIMPORT void jl_get_llvmf_defn(jl_llvmf_dump_t* dump, jl_method_instance_t *linfo, size_t world, char getwrapper, char optimize, const jl_cgparams_t params);
-JL_DLLIMPORT jl_value_t *jl_dump_fptr_asm(uint64_t fptr, char raw_mc, const char* asm_variant, const char *debuginfo, char binary);
+JL_DLLIMPORT jl_value_t *jl_dump_fptr_asm(uint64_t fptr, char emit_mc, const char* asm_variant, const char *debuginfo, char binary);
 JL_DLLIMPORT jl_value_t *jl_dump_function_ir(jl_llvmf_dump_t *dump, char strip_ir_metadata, char dump_module, const char *debuginfo);
-JL_DLLIMPORT jl_value_t *jl_dump_function_asm(jl_llvmf_dump_t *dump, char raw_mc, const char* asm_variant, const char *debuginfo, char binary);
+JL_DLLIMPORT jl_value_t *jl_dump_function_asm(jl_llvmf_dump_t *dump, char emit_mc, const char* asm_variant, const char *debuginfo, char binary, char raw);
 
 JL_DLLIMPORT void *jl_create_native(jl_array_t *methods, LLVMOrcThreadSafeModuleRef llvmmod, const jl_cgparams_t *cgparams, int policy, int imaging_mode, int cache, size_t world);
 JL_DLLIMPORT void jl_dump_native(void *native_code,
diff --git a/stdlib/InteractiveUtils/src/codeview.jl b/stdlib/InteractiveUtils/src/codeview.jl
@@ -169,16 +169,15 @@ const OC_MISMATCH_WARNING =
 # Printing code representations in IR and assembly
 
 function _dump_function(@nospecialize(f), @nospecialize(t), native::Bool, wrapper::Bool,
-                        strip_ir_metadata::Bool, dump_module::Bool, syntax::Symbol,
-                        optimize::Bool, debuginfo::Symbol, binary::Bool,
-                        safepoints::Bool=false)
+                        raw::Bool, dump_module::Bool, syntax::Symbol,
+                        optimize::Bool, debuginfo::Symbol, binary::Bool)
         params = CodegenParams(debug_info_kind=Cint(0),
-                               safepoint_on_entry=safepoints)
-        _dump_function(f, t, native, wrapper, strip_ir_metadata, dump_module, syntax,
+                               safepoint_on_entry=raw)
+        _dump_function(f, t, native, wrapper, raw, dump_module, syntax,
                        optimize, debuginfo, binary, params)
 end
 function _dump_function(@nospecialize(f), @nospecialize(t), native::Bool, wrapper::Bool,
-                        strip_ir_metadata::Bool, dump_module::Bool, syntax::Symbol,
+                        raw::Bool, dump_module::Bool, syntax::Symbol,
                         optimize::Bool, debuginfo::Symbol, binary::Bool, params::CodegenParams)
     ccall(:jl_is_in_pure_context, Bool, ()) && error("code reflection cannot be used from generated functions")
     if isa(f, Core.Builtin)
@@ -189,21 +188,21 @@ function _dump_function(@nospecialize(f), @nospecialize(t), native::Bool, wrappe
     if !isa(f, Core.OpaqueClosure)
         world = Base.get_world_counter()
         match = Base._which(signature_type(f, t); world)
-        linfo = Core.Compiler.specialize_method(match)
+        mi = Core.Compiler.specialize_method(match)
         # TODO: use jl_is_cacheable_sig instead of isdispatchtuple
-        isdispatchtuple(linfo.specTypes) || (warning = GENERIC_SIG_WARNING)
+        isdispatchtuple(mi.specTypes) || (warning = GENERIC_SIG_WARNING)
     else
         world = UInt64(f.world)
         if Core.Compiler.is_source_inferred(f.source.source)
             # OC was constructed from inferred source. There's only one
             # specialization and we can't infer anything more precise either.
             world = f.source.primary_world
-            linfo = f.source.specializations::Core.MethodInstance
+            mi = f.source.specializations::Core.MethodInstance
             Core.Compiler.hasintersect(typeof(f).parameters[1], t) || (warning = OC_MISMATCH_WARNING)
         else
-            linfo = Core.Compiler.specialize_method(f.source, Tuple{typeof(f.captures), t.parameters...}, Core.svec())
-            actual = isdispatchtuple(linfo.specTypes)
-            isdispatchtuple(linfo.specTypes) || (warning = GENERIC_SIG_WARNING)
+            mi = Core.Compiler.specialize_method(f.source, Tuple{typeof(f.captures), t.parameters...}, Core.svec())
+            actual = isdispatchtuple(mi.specTypes)
+            isdispatchtuple(mi.specTypes) || (warning = GENERIC_SIG_WARNING)
         end
     end
     # get the code for it
@@ -218,25 +217,24 @@ function _dump_function(@nospecialize(f), @nospecialize(t), native::Bool, wrappe
         end
         if dump_module
             # we want module metadata, so use LLVM to generate assembly output
-            str = _dump_function_linfo_native(linfo, world, wrapper, syntax, debuginfo, binary, params)
+            str = _dump_function_native_assembly(mi, world, wrapper, syntax, debuginfo, binary, raw, params)
         else
-            # if we don't want the module metadata, just disassemble what our JIT has.
-            # TODO: make it possible to hide the safepoint prologue here too.
-            #       once that works, it would be good to default to dump_module=false
-            #       just like code_llvm does.
-            str = _dump_function_linfo_native(linfo, world, wrapper, syntax, debuginfo, binary)
+            # if we don't want the module metadata, just disassemble what our JIT has
+            str = _dump_function_native_disassembly(mi, world, wrapper, syntax, debuginfo, binary)
         end
     else
-        str = _dump_function_linfo_llvm(linfo, world, wrapper, strip_ir_metadata, dump_module, optimize, debuginfo, params)
+        str = _dump_function_llvm(mi, world, wrapper, !raw, dump_module, optimize, debuginfo, params)
     end
     str = warning * str
     return str
 end
 
-function _dump_function_linfo_native(linfo::Core.MethodInstance, world::UInt, wrapper::Bool, syntax::Symbol, debuginfo::Symbol, binary::Bool)
-    str = ccall(:jl_dump_method_asm, Ref{String},
-                (Any, UInt, Bool, Bool, Ptr{UInt8}, Ptr{UInt8}, Bool),
-                linfo, world, false, wrapper, syntax, debuginfo, binary)
+function _dump_function_native_disassembly(mi::Core.MethodInstance, world::UInt,
+                                           wrapper::Bool, syntax::Symbol,
+                                           debuginfo::Symbol, binary::Bool)
+    str = @ccall jl_dump_method_asm(mi::Any, world::UInt, false::Bool, wrapper::Bool,
+                                    syntax::Ptr{UInt8}, debuginfo::Ptr{UInt8},
+                                    wrapper::Bool)::Ref{String}
     return str
 end
 
@@ -245,27 +243,30 @@ struct LLVMFDump
     f::Ptr{Cvoid} # opaque
 end
 
-function _dump_function_linfo_native(linfo::Core.MethodInstance, world::UInt, wrapper::Bool, syntax::Symbol, debuginfo::Symbol, binary::Bool, params::CodegenParams)
+function _dump_function_native_assembly(mi::Core.MethodInstance, world::UInt,
+                                        wrapper::Bool, syntax::Symbol, debuginfo::Symbol,
+                                        binary::Bool, raw::Bool, params::CodegenParams)
     llvmf_dump = Ref{LLVMFDump}()
-    ccall(:jl_get_llvmf_defn, Cvoid, (Ptr{LLVMFDump}, Any, UInt, Bool, Bool, CodegenParams), llvmf_dump, linfo, world, wrapper, true, params)
+    @ccall jl_get_llvmf_defn(llvmf_dump::Ptr{LLVMFDump},mi::Any, world::UInt, wrapper::Bool,
+                             true::Bool, params::CodegenParams)::Cvoid
     llvmf_dump[].f == C_NULL && error("could not compile the specified method")
-    str = ccall(:jl_dump_function_asm, Ref{String},
-                (Ptr{LLVMFDump}, Bool, Ptr{UInt8}, Ptr{UInt8}, Bool),
-                llvmf_dump, false, syntax, debuginfo, binary)
+    str = @ccall jl_dump_function_asm(llvmf_dump::Ptr{LLVMFDump}, false::Bool,
+                                      syntax::Ptr{UInt8}, debuginfo::Ptr{UInt8},
+                                      binary::Bool, raw::Bool)::Ref{String}
     return str
 end
 
-function _dump_function_linfo_llvm(
-        linfo::Core.MethodInstance, world::UInt, wrapper::Bool,
+function _dump_function_llvm(
+        mi::Core.MethodInstance, world::UInt, wrapper::Bool,
         strip_ir_metadata::Bool, dump_module::Bool,
         optimize::Bool, debuginfo::Symbol,
         params::CodegenParams)
     llvmf_dump = Ref{LLVMFDump}()
-    ccall(:jl_get_llvmf_defn, Cvoid, (Ptr{LLVMFDump}, Any, UInt, Bool, Bool, CodegenParams), llvmf_dump, linfo, world, wrapper, optimize, params)
+    @ccall jl_get_llvmf_defn(llvmf_dump::Ptr{LLVMFDump}, mi::Any, world::UInt,
+                             wrapper::Bool, optimize::Bool, params::CodegenParams)::Cvoid
     llvmf_dump[].f == C_NULL && error("could not compile the specified method")
-    str = ccall(:jl_dump_function_ir, Ref{String},
-                (Ptr{LLVMFDump}, Bool, Bool, Ptr{UInt8}),
-                llvmf_dump, strip_ir_metadata, dump_module, debuginfo)
+    str = @ccall jl_dump_function_ir(llvmf_dump::Ptr{LLVMFDump}, strip_ir_metadata::Bool,
+                                     dump_module::Bool, debuginfo::Ptr{UInt8})::Ref{String}
     return str
 end
 
@@ -282,7 +283,7 @@ Keyword argument `debuginfo` may be one of source (default) or none, to specify
 """
 function code_llvm(io::IO, @nospecialize(f), @nospecialize(types), raw::Bool,
                    dump_module::Bool=false, optimize::Bool=true, debuginfo::Symbol=:default)
-    d = _dump_function(f, types, false, false, !raw, dump_module, :intel, optimize, debuginfo, false, raw)
+    d = _dump_function(f, types, false, false, raw, dump_module, :intel, optimize, debuginfo, false)
     if highlighting[:llvm] && get(io, :color, false)::Bool
         print_llvm(io, d)
     else
@@ -304,14 +305,14 @@ generic function and type signature to `io`.
 * Specify verbosity of code comments by setting `debuginfo` to `:source` (default) or `:none`.
 * If `binary` is `true`, also print the binary machine code for each instruction precedented by an abbreviated address.
 * If `dump_module` is `false`, do not print metadata such as rodata or directives.
-* If `raw` is `false`, unintesting instructions (like the safepoint function prologue) are elided.
+* If `raw` is `false`, uninteresting instructions (like the safepoint function prologue) are elided.
 
 See also: [`@code_native`](@ref), [`code_llvm`](@ref), [`code_typed`](@ref) and [`code_lowered`](@ref)
 """
 function code_native(io::IO, @nospecialize(f), @nospecialize(types=Base.default_tt(f));
                      dump_module::Bool=true, syntax::Symbol=:intel, raw::Bool=false,
                      debuginfo::Symbol=:default, binary::Bool=false)
-    d = _dump_function(f, types, true, false, false, dump_module, syntax, true, debuginfo, binary, raw)
+    d = _dump_function(f, types, true, false, raw, dump_module, syntax, true, debuginfo, binary)
     if highlighting[:native] && get(io, :color, false)::Bool
         print_native(io, d)
     else
diff --git a/test/compiler/codegen.jl b/test/compiler/codegen.jl
@@ -17,7 +17,7 @@ end
 
 # The tests below assume a certain format and safepoint_on_entry=true breaks that.
 function get_llvm(@nospecialize(f), @nospecialize(t), raw=true, dump_module=false, optimize=true)
-    d = InteractiveUtils._dump_function(f, t, false, false, !raw, dump_module, :att, optimize, :none, false, false)
+    d = InteractiveUtils._dump_function(f, t, false, false, raw, dump_module, :att, optimize, :none, false)
     sprint(print, d)
 end
 
diff --git a/test/reflection.jl b/test/reflection.jl
@@ -909,9 +909,9 @@ _test_at_locals2(1,1,0.5f0)
     f31687_parent() = f31687_child(0)
     params = Base.CodegenParams()
     _dump_function(f31687_parent, Tuple{},
-                   #=native=#false, #=wrapper=#false, #=strip=#false,
+                   #=native=#false, #=wrapper=#false, #=raw=#true,
                    #=dump_module=#true, #=syntax=#:att, #=optimize=#false, :none,
-                   #=binary=#false, #=safepoint=#false)
+                   #=binary=#false)
 end
 
 @test nameof(Any) === :Any
