Skip to content

Fixes for 1.13 #693

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

Merged
merged 5 commits into from
May 23, 2025
Merged

Fixes for 1.13 #693

merged 5 commits into from
May 23, 2025

Conversation

maleadt
Copy link
Member

@maleadt maleadt commented May 8, 2025

  • Fallout from replace incorrect Method.deleted_world with more useful Method.dispatch_status enum JuliaLang/julia#58291

     RROR: LoadError: FieldError: type Method has no field `deleted_world`, available fields: `name`, `module`, `file`, `line`, `dispatch_status`, `primary_world`, `sig`, `specializations`, `speckeyset`, `slot_syms`, `external_mt`, `source`, `debuginfo`, `unspecialized`, `generator`, `roots`, `root_blocks`, `nroots_sysimg`, `ccallable`, `invokes`, `recursion_relation`, `nargs`, `called`, `nospecialize`, `nkw`, `isva`,   `is_for_opaque_closure`, `nospecializeinfer`, `did_scan_source`, `constprop`, `max_varargs`, `purity`
    Stacktrace:
      [1] getproperty
        @ ./Base_compiler.jl:57 [inlined]
      [2] compile_method_instance(job::GPUCompiler.CompilerJob)
        @ GPUCompiler ~/Julia/pkg/GPUCompiler/src/jlgen.jl:590
    
  • Fallout from trimming: Support finalizers JuliaLang/julia#58014

    ERROR: LoadError: MethodError: no method matching collectinvokes!(::Vector{Core.CodeInstance}, ::Core.CodeInfo)
    The function `collectinvokes!` exists, but no method is defined for this combination of argument types.
    
    Closest candidates are:
      collectinvokes!(::Compiler.CompilationQueue, ::Core.CodeInfo, ::Vector{Compiler.VarState}; invokelatest_queue)
       @ Base ~/Julia/depot/juliaup/julia-nightly/share/julia/Compiler/src/typeinfer.jl:1429
    
    Stacktrace:
        ] ci_cache_populate(interp::GPUCompiler.GPUInterpreter, cache::Compiler.WorldView{Compiler.InternalCodeCache}, mi::Core.MethodInstance, min_world::UInt64, max_world::UInt64)
        @ GPUCompiler ~/Julia/pkg/GPUCompiler/src/jlgen.jl:504
    
  • Fallout from codegen: add a pass for late conversion of known modify ops to call atomicrmw JuliaLang/julia#57010

    native: Test Failed at /home/tim/Julia/pkg/GPUCompiler/test/native.jl:276
      Expression: occursin("%safepoint", ir)
        valuated: occursin("%safepoint", "; ModuleID = 'start'\nsource_filename = \"start\"\ntarget datalayout = \"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128\"\ntarget triple = \"x86_64-linux-gnu\"\n\n;  @ operators.jl:577 within `identity`\ndefine void @julia_identity_10907() local_unnamed_addr #0 {\ntop:\n;  @ operators.jl within `identity`\n  ret void\n}\n\nattributes #0 = { \"frame-pointer\"=\"all\" }\n\n!llvm.module.flags = !{!0, !1}\n\n!0 = !{i32 2, !\"Dwarf Version\", i32 4}\n!1 = !{i32 2, !\"Debug Info Version\", i32 3}\n")
      Stacktrace:
       [1] record(ts::Test.DefaultTestSet, t::Union{Test.Error, Test.Fail}; print_result::Bool)
         @ Test ~/Julia/depot/juliaup/julia-nightly/share/julia/stdlib/v1.13/Test/src/Test.jl:1177
       [2] record(ts::Test.DefaultTestSet, t::Union{Test.Error, Test.Fail})
         @ Test ~/Julia/depot/juliaup/julia-nightly/share/julia/stdlib/v1.13/Test/src/Test.jl:1169
       [3] top-level scope
         @ ~/Julia/pkg/GPUCompiler/test/runtests.jl:364
       [4] include(mapexpr::Function, mod::Module, _path::String)
         @ Base ./Base.jl:310
       [5] top-level scope
         @ none:6
       [6] eval(m::Module, e::Any)
         @ Core ./boot.jl:489
       [7] exec_options(opts::Base.JLOptions)
         @ Base ./client.jl:297
       [8] _start()
         @ Base ./client.jl:564
    

    Marked the test as broken for now; the change in jl_emit_native seems really ad-hoc.

  • Debug info version mismatch

    warning: linking module flags 'Dwarf Version': IDs have conflicting values ('i32 4' from kernel with 'i32 2' from start)
    
  • Data layout mismatch

    warning: Linking two modules of different data layouts: '' is 'e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-ni:10:11:12:13-G1' whereas 'start' is 'e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-ni:10:11:12:13'
    
  • Changes in bindings: global variables now don't get initializer anymore, so we can't reconstruct the object pointer. This was broken by codegen: add a pass for late conversion of known modify ops to call atomicrmw JuliaLang/julia#57010, see codegen: add a pass for late conversion of known modify ops to call atomicrmw JuliaLang/julia#57010 (review)

    ┌ Error: Failed to validate error message
    │ InvalidIRError: compiling MethodInstance for kernel() resulted in invalid LLVM IR
    │ Reason: unsupported use of an undefined name
    │ Stacktrace:
    │  [1] kernel
    │    @ ~/Julia/pkg/GPUCompiler/test/native.jl:445
    │ Hint: catch this exception as `err` and call `code_typed(err; interactive = true)` to introspect the erroneous code with Cthulhu.jl
    └ @ Main ~/Julia/pkg/GPUCompiler/test/helpers/test.jl:15
    

    Looks like the way bindings are resolved has completely changed. Take this simple example:

    function kernel()
      undefined
      return
    end

    This used to emit something like:

    @"jl_global#572.jit" = private alias ptr, inttoptr (i64 134172302677008 to ptr)
    @"jl_sym#undefined#573.jit" = private alias ptr, inttoptr (i64 134172460581032 to ptr)
    
    ; Function Signature: kernel()
    ;  @ REPL[1]:1 within `kernel`
    define void @julia_kernel_570() #0 {
    top:
    ;  @ REPL[1]:2 within `kernel`
      %Main.undefined.cached = load atomic ptr, ptr @jl_binding_ptr unordered, align 8
      %iscached.not = icmp eq ptr %Main.undefined.cached, null
      br i1 %iscached.not, label %notfound, label %found
    
    notfound:                                         ; preds = %top
      %Main.undefined.found = call ptr @ijl_get_binding_or_error(ptr nonnull @"jl_global#572.jit", ptr nonnull @"jl_sym#undefined#573.jit")
      store atomic ptr %Main.undefined.found, ptr @jl_binding_ptr release, align 8
      br label %found
    
    found:                                            ; preds = %notfound, %top
      %Main.undefined = phi ptr [ %Main.undefined.cached, %top ], [ %Main.undefined.found, %notfound ]
      %undefined.checked = load atomic ptr, ptr %Main.undefined unordered, align 8
      %.not = icmp eq ptr %undefined.checked, null
      br i1 %.not, label %err, label %ok
    
    err:                                              ; preds = %found
      call void @ijl_undefined_var_error(ptr nonnull @"jl_sym#undefined#573.jit", ptr nonnull @"jl_global#572.jit")
      unreachable
    
    ok:                                               ; preds = %found
    ;  @ REPL[1]:3 within `kernel`
      ret void
    }

    ... which GPUCompiler.jl could pry a pointer to the binding from. Now we get:

    @"*Main.##280.undefined#41043" = internal global ptr null, !julia.constgv !0
    @"jl_sym#undefined#41044" = internal global ptr null, !julia.constgv !0
    @"jl_global#41045" = internal global ptr null, !julia.constgv !0
    
    define void @julia_kernel_41041() #0 !dbg !5 {
    top:
      %pgcstack = call ptr @julia.get_pgcstack()
      %"*Main.##280.undefined#41043" = load ptr, ptr @"*Main.##280.undefined#41043"
      %0 = call ptr addrspace(10) @jl_get_binding_value_seqcst(ptr %"*Main.##280.undefined#41043")
      %1 = icmp ne ptr addrspace(10) %0, null
      br i1 %1, label %ok, label %err
    
    err:
      %"jl_sym#undefined#41044" = load ptr, ptr @"jl_sym#undefined#41044"
      %2 = addrspacecast ptr %"jl_sym#undefined#41044" to ptr addrspace(12)
      %"jl_global#41045" = load ptr, ptr @"jl_global#41045"
      %3 = addrspacecast ptr %"jl_global#41045" to ptr addrspace(12)
      call void @ijl_undefined_var_error(ptr addrspace(12) %2, ptr addrspace(12) %3)
      unreachable
    
    ok:
      ret void
    }

    I guess we're missing some things getting linked here.

  • Changes in dynamic invocations

    ┌ Error: Failed to validate error message
    │ InvalidIRError: compiling MethodInstance for Main.var"##287".kernel(::Int64, ::Ptr{Int64}) resulted in invalid LLVM IR
    │ Reason: unsupported dynamic function invocation
    │ Stacktrace:
    │  [1] kernel
    │    @ ~/Julia/pkg/GPUCompiler/test/native.jl:462
    │ Hint: catch this exception as `err` and call `code_typed(err; interactive = true)` to introspect the erroneous code with Cthulhu.jl
    └ @ Main ~/Julia/pkg/GPUCompiler/test/helpers/test.jl:15
    

Copy link

codecov bot commented May 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.07%. Comparing base (bdf390a) to head (534dd85).
Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #693      +/-   ##
==========================================
- Coverage   73.70%   72.07%   -1.64%     
==========================================
  Files          24       24              
  Lines        3472     3484      +12     
==========================================
- Hits         2559     2511      -48     
- Misses        913      973      +60     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@maleadt maleadt marked this pull request as ready for review May 23, 2025 09:15
Copy link
Contributor

Your PR requires formatting changes to meet the project's style guidelines.
Please consider running Runic (git runic master) to apply these changes.

Click here to view the suggested changes.
diff --git a/src/jlgen.jl b/src/jlgen.jl
index f98780f..5754896 100644
--- a/src/jlgen.jl
+++ b/src/jlgen.jl
@@ -702,14 +702,20 @@ function compile_method_instance(@nospecialize(job::CompilerJob))
         # Since Julia 1.13, the caller is responsible for initializing global variables that
         # point to global values or bindings with their address in memory.
         num_gvars = Ref{Csize_t}(0)
-        @ccall jl_get_llvm_gvs(native_code::Ptr{Cvoid}, num_gvars::Ptr{Csize_t},
-                               C_NULL::Ptr{Cvoid})::Nothing
+        @ccall jl_get_llvm_gvs(
+            native_code::Ptr{Cvoid}, num_gvars::Ptr{Csize_t},
+            C_NULL::Ptr{Cvoid}
+        )::Nothing
         gvs = Vector{Ptr{LLVM.API.LLVMOpaqueValue}}(undef, num_gvars[])
-        @ccall jl_get_llvm_gvs(native_code::Ptr{Cvoid}, num_gvars::Ptr{Csize_t},
-                               gvs::Ptr{LLVM.API.LLVMOpaqueValue})::Nothing
+        @ccall jl_get_llvm_gvs(
+            native_code::Ptr{Cvoid}, num_gvars::Ptr{Csize_t},
+            gvs::Ptr{LLVM.API.LLVMOpaqueValue}
+        )::Nothing
         inits = Vector{Ptr{Cvoid}}(undef, num_gvars[])
-        @ccall jl_get_llvm_gv_inits(native_code::Ptr{Cvoid}, num_gvars::Ptr{Csize_t},
-                                    inits::Ptr{Cvoid})::Nothing
+        @ccall jl_get_llvm_gv_inits(
+            native_code::Ptr{Cvoid}, num_gvars::Ptr{Csize_t},
+            inits::Ptr{Cvoid}
+        )::Nothing
 
         for (gv_ref, init) in zip(gvs, inits)
             gv = GlobalVariable(gv_ref)
diff --git a/test/native.jl b/test/native.jl
index 1821761..794a0e3 100644
--- a/test/native.jl
+++ b/test/native.jl
@@ -273,9 +273,9 @@ end
     @test !occursin("%safepoint", ir)
 
     ir = sprint(io->Native.code_llvm(io, identity, Tuple{Nothing}; entry_safepoint=true, optimize=false, dump_module=true))
-    @test occursin("%safepoint", ir) broken=(VERSION >= v"1.13.0-DEV.533")
-    # XXX: broken by JuliaLang/julia#57010,
-    #      see https://github.com/JuliaLang/julia/pull/57010/files#r2079576894
+        @test occursin("%safepoint", ir) broken = (VERSION >= v"1.13.0-DEV.533")
+        # XXX: broken by JuliaLang/julia#57010,
+        #      see https://github.com/JuliaLang/julia/pull/57010/files#r2079576894
 end
 
 @testset "always_inline" begin

Switches to putting globals in AS 1, as LLVM now defaults to
starting with version 19.
@maleadt maleadt merged commit dcc2038 into master May 23, 2025
21 of 22 checks passed
@maleadt maleadt deleted the tb/1.13 branch May 23, 2025 12:52
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