Fix imaging value for generating sysimages for multiple targets #48809

ven-k · 2023-02-27T11:08:34Z

Fixes imaging_default function, so that sysimages for multiple architectures can be built without setting pkgimages=yes

Current issue:

For versions >1.9-dev,
For a use-case like:

julia> sysimage= unsafe_string(Base.JLOptions().image_file)

julia>  run(`./julia
               --sysimage=$sysimage
               --cpu-target='generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)'
               --output-o=mock.o mock.jl
               --pkgimages=no`)

fatal: error thrown and no exception handler available.
ErrorException("More than one command line CPU targets specified without a `--output-` flag specified")

[30365] signal (11.1): Segmentation fault
in expression starting at none:0

This is because, with pkgimages=no, even if output-o is passed, the imaging_default returns false. This in-turn causes issues while generating sysimages for multiple cpu-targets

After this PR, these work:

Sysimages with multiple targets and no pkgimages

julia> run(`julia --sysimage=$sysimage --cpu-target='generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)' --output-o=mock.o mock.jl --pkgimages=no`)

Pkgimages=yes and multiple targets.
From the docs, there should be a unified cache. It's unclear to me if the current change results an issue in that regard. An alternative would be changing to return jl_options.image_codegen || jl_generating_output().

julia> run(`julia --cpu-target='generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)' --pkgimages=yes`)

vchuravy · 2023-02-27T13:37:29Z

Could you add a test here?

vchuravy · 2023-02-27T19:34:33Z

test/cmdlineargs.jl

@@ -597,6 +597,34 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
 "Int(Base.JLOptions().fast_math)"`)) == JL_OPTIONS_FAST_MATH_DEFAULT
 end

+ # --pkgimages with multiple cpu targets
+ @testset let v = readchomperrors(`$exename


This probably needs a guard for x86_64 or a different list for supported architectures.

vchuravy · 2023-02-27T19:37:48Z

test/cmdlineargs.jl

+ end
+
+ @test success(`$exename
+ --cpu-target='generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)'


Okay this should be propagated to the target process.

julia/base/util.jl

Line 171 in a23b29c

cpu_target = unsafe_string(opts.cpu_target)

As should JULIA_CPU_TARGET.

julia/base/loading.jl

Line 2071 in a23b29c

cpu_target = get(ENV, "JULIA_CPU_TARGET", nothing)

vtjnash · 2023-02-27T21:59:16Z

test/cmdlineargs.jl

@@ -597,6 +597,34 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
 "Int(Base.JLOptions().fast_math)"`)) == JL_OPTIONS_FAST_MATH_DEFAULT
 end

+ # --pkgimages with multiple cpu targets
+ @testset let v = readchomperrors(`$exename
+ --cpu-target='generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)'


Suggested change

--cpu-target='generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)'

--cpu-target='generic;generic'

vtjnash · 2023-02-27T22:00:31Z

test/cmdlineargs.jl

+ --cpu-target='generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)'
+ --output-o=$object_file $outputo_file
+ --pkgimages=no`)
+ end


this test sounds slow, and should not be included without further reconsideration of performance

Yes. I've noticed that this test adds ~160s. It is too much time for 1 single test.
So, for easy dropping, I've added it in a dedicated commit.

I couldn't think of any other simpler test for output-o, which is faster, though.

@vtjnash How about this: we intentionally don't pass the sysimage arg. When it fails we know everything before that passed, including our case. This takes only 0.05s

# Sysimage with multiple cpu targets @testset "Sysimage for multiple microarchitectures" begin julia_path = joinpath(Sys.BINDIR, Base.julia_exename()) outputo_file = tempname() write(outputo_file, "1") object_file = tempname() * ".o" # This is to test that even with `pkgimages=no`, we can create object file # with multiple cpu-targets # The cmd is checked for --object-o as soon as it is run. So, to avoid long # testing times, intentionally don't pass sysimage; when we reach the # corresponding error, we know that `check_cmdline` has already passed let v = readchomperrors(`$julia_path --cpu-target='native;native' --output-o=$object_file $outputo_file --pkgimages=no`) @test v[1] == false @test v[2] == "" err = first(split(v[3], "\n\n")) @test err != "fatal: error thrown and no exception handler available.\nErrorException(\"More than one command line CPU targets specified without a `--output-` flag specified\")" @test err == "ERROR: File \"boot.jl\" not found" end end

yes, though let's do @test !contains(err, "More than one command line CPU targets specified") so it will still work if other minor aspects change (though the second test for == will also catch those)

DilumAluthge · 2023-02-28T19:11:23Z

This PR is marked as merge me, but it seems that there are still some unresolved conversations. So I'll remove the label.

@vtjnash or @vchuravy Can you add the merge me label back once this PR is ready?

DilumAluthge · 2023-02-28T20:00:00Z

Also, looks like the PowerPC build is broken @vchuravy

DilumAluthge · 2023-03-04T22:19:28Z

@vchuravy Do you want me to mark the PowerPC build as "allow fail"?

(The PowerPC tests are already marked as "allow fail", but currently the PowerPC build is marked as "regular".)

vchuravy · 2023-03-05T18:08:33Z

No the PPC build is not allowed to fail.

Edit: I do not see how this change got impacted the architecture, but we should not downgrade the platform.

DilumAluthge · 2023-03-30T21:29:38Z

It seems like this PR is a release blocker for 1.9. Are we willing to let a broken PowerPC build block this PR? The website says that PowerPC is a Tier 3 platform.

vchuravy · 2023-03-31T19:46:10Z

That's a good observation. This PR also seems to change JIT compliation when --pkgimages=yes

 fence syncscope("singlethread") seq_cst                                                                                                                                                                                                                                                                                     
;  @ REPL[1]:1 within `my_cd`                                                                                                                                                                                                                                                                                                 
  %11 = load volatile i64, i64* %safepoint, align 8                                                                                                                                                                                                                                                                           
  fence syncscope("singlethread") seq_cst                                                                                                                                                                                                                                                                                     
;  @ REPL[1]:2 within `my_cd`                                                                                                                                                                                                                                                                                                 
; ┌ @ c.jl:234 within `unsafe_convert` @ pointer.jl:58                                                                                                                                                                                                                                                                        
   %12 = load {}**, {}*** bitcast ({}** @"jl_sym#.#2" to {}***), align 8                                                                                                                                                                                                                                                      
   %13 = getelementptr inbounds {}*, {}** %12, i64 3
   %14 = ptrtoint {}** %13 to i64
; └
  %15 = load atomic i32 (i64, i32)*, i32 (i64, i32)** bitcast (void ()** @jlplt_open_176_got to i32 (i64, i32)**) unordered, align 8
  %16 = call i32 %15(i64 %14, i32 0)

versus

  fence syncscope("singlethread") seq_cst                                                                                                                                                                                                                                                                                     
;  @ REPL[1]:1 within `my_cd`                                                                                                                                                                                                                                                                                                 
  %11 = load volatile i64, i64* %safepoint, align 8                                                                                                                                                                                                                                                                           
  fence syncscope("singlethread") seq_cst                                                                                                                                                                                                                                                                                     
;  @ REPL[1]:2 within `my_cd`                                                                                                                                                                                                                                                                                                 
  %12 = call i32 inttoptr (i64 140735334611648 to i32 (i64, i32)*)(i64 140735201562464, i32 0)                                                                                                                                                                                                                                
;  @ REPL[1]:4 within `my_cd`                                                                                                                                                                                                                                                                                                 
; ┌ @ c.jl:215 within `unsafe_convert` @ pointer.jl:60

src/jitlayers.h

staticfloat · 2023-04-11T22:09:01Z

@ven-k can you please confirm that this PR fixes your original issue?

vchuravy

LGTM, modulo checking that we are not using imaging mode in normal JIT mode.

staticfloat · 2023-04-12T02:25:41Z

LGTM, modulo checking that we are not using imaging mode in normal JIT mode.

I ran a check with @code_native of a few different functions (including ones that generated AVX instructions) with both --pkgimages=yes and --pkgimages=no. I couldn't find any meaningful difference, so I'm calling it good.

ven-k · 2023-04-12T16:06:15Z

With this I can pass --cpu-targets and generate target-compatible sysimages.

Note that when it is added to env via JULIA_CPU_TARGET (Currently PackageCompiler does that for >=v1.9), on relocating I got a segfault. (But considering passing the --cpu-targets works, not a blocking issue for my use case)

./julia-fc4b079f69/bin/julia -J fc4b079f69_8.so

[5504] signal (7.2): Bus error
in expression starting at none:0
unknown function (ip: 0x7fb2ea3c75d3)
unknown function (ip: 0x7fb2ea3ab428)
unknown function (ip: 0x7fb2ea3ae61a)
unknown function (ip: 0x7fb2ea3b9d46)
_dl_catch_exception at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x7fb2ea3b9609)
unknown function (ip: 0x7fb2ea39934b)
_dl_catch_exception at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
_dl_catch_error at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x7fb2ea399b58)
dlopen at /lib/x86_64-linux-gnu/libdl.so.2 (unknown line)
ijl_load_dynamic_library at /cache/build/default-amdci5-0/julialang/julia-master/src/dlload.c:355
ijl_preload_sysimg_so at /cache/build/default-amdci5-0/julialang/julia-master/src/staticdata.c:2641
_finish_julia_init at /cache/build/default-amdci5-0/julialang/julia-master/src/init.c:827
julia_init at /cache/build/default-amdci5-0/julialang/julia-master/src/init.c:819
jl_repl_entrypoint at /cache/build/default-amdci5-0/julialang/julia-master/src/jlapi.c:711
main at /cache/build/default-amdci5-0/julialang/julia-master/cli/loader_exe.c:58
__libc_start_main at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x401098)
Allocations: 0 (Pool: 0; Big: 0); GC: 0

[5504] signal (7.2): Bus error
in expression starting at none:0
unknown function (ip: 0x7fb2ea3c75d3)
unknown function (ip: 0x7fb2ea3ab428)
unknown function (ip: 0x7fb2ea3ae61a)
unknown function (ip: 0x7fb2ea3b9d46)
_dl_catch_exception at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x7fb2ea3b9609)
unknown function (ip: 0x7fb2ea39934b)
_dl_catch_exception at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
_dl_catch_error at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x7fb2ea399b58)
dlopen at /lib/x86_64-linux-gnu/libdl.so.2 (unknown line)
ijl_load_dynamic_library at /cache/build/default-amdci5-0/julialang/julia-master/src/dlload.c:355
ijl_preload_sysimg_so at /cache/build/default-amdci5-0/julialang/julia-master/src/staticdata.c:2641
_finish_julia_init at /cache/build/default-amdci5-0/julialang/julia-master/src/init.c:827
julia_init at /cache/build/default-amdci5-0/julialang/julia-master/src/init.c:819
jl_repl_entrypoint at /cache/build/default-amdci5-0/julialang/julia-master/src/jlapi.c:711
main at /cache/build/default-amdci5-0/julialang/julia-master/cli/loader_exe.c:58
__libc_start_main at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x401098)
Allocations: 0 (Pool: 0; Big: 0); GC: 0
Bus error (core dumped)

DilumAluthge · 2023-04-12T23:35:34Z

@vchuravy @staticfloat Would it be possible to make JULIA_CPU_TARGET support the same values as --cpu-targets?

Fix imaging value for generating sysimages for multiple targets (cherry picked from commit fc4b079)

ven-k mentioned this pull request Feb 27, 2023

With 1.9.0-beta4, sysimages aren't built for the specified JULIA_CPU_TARGET JuliaLang/PackageCompiler.jl#790

Closed

KristofferC added the backport 1.9 Change should be backported to release-1.9 label Feb 27, 2023

KristofferC requested a review from vchuravy February 27, 2023 11:23

vtjnash added the status:merge me PR is reviewed. Merge when all tests are passing label Feb 27, 2023

vchuravy reviewed Feb 27, 2023

View reviewed changes

vchuravy requested a review from vtjnash February 27, 2023 19:38

vtjnash reviewed Feb 27, 2023

View reviewed changes

DilumAluthge removed the status:merge me PR is reviewed. Merge when all tests are passing label Feb 28, 2023

DilumAluthge added the kind:bugfix This change fixes an existing bug label Feb 28, 2023

DilumAluthge requested review from vtjnash and vchuravy February 28, 2023 20:23

ven-k force-pushed the vkb/imageing-default branch from 70b36e6 to ceb107e Compare March 1, 2023 05:35

KristofferC mentioned this pull request Mar 3, 2023

Backports for 1.9.0-rc1 #48733

Merged

50 tasks

ven-k force-pushed the vkb/imageing-default branch from ceb107e to 01e97f7 Compare March 4, 2023 05:36

KristofferC mentioned this pull request Mar 7, 2023

Backports for 1.9.0-rc2 #48935

Merged

52 tasks

ven-k force-pushed the vkb/imageing-default branch from 01e97f7 to 9b28ad0 Compare March 23, 2023 05:19

vchuravy force-pushed the vkb/imageing-default branch from 9b28ad0 to 31b155b Compare March 29, 2023 22:16

KristofferC added this to the 1.9 milestone Mar 30, 2023

This comment was marked as off-topic.

Sign in to view

vchuravy mentioned this pull request Mar 31, 2023

Make :open ccall use variadic cconv #49212

Merged

vchuravy reviewed Mar 31, 2023

View reviewed changes

src/jitlayers.h Outdated Show resolved Hide resolved

KristofferC mentioned this pull request Apr 9, 2023

Backports for 1.9.0 #49302

Merged

26 tasks

ven-k and others added 3 commits April 11, 2023 15:42

fix: return true while either generating the output "or" using pkgimages

d86285d

test: pkgimages and object files with multiple cpu targets

4f4842c

Incorporate review feedback

3fa1bcb

staticfloat force-pushed the vkb/imageing-default branch from b6fd51b to 3fa1bcb Compare April 11, 2023 15:42

Adjust tests

acd51cf

staticfloat requested a review from vchuravy April 11, 2023 22:08

vchuravy approved these changes Apr 11, 2023

View reviewed changes

staticfloat merged commit fc4b079 into JuliaLang:master Apr 12, 2023

KristofferC pushed a commit that referenced this pull request Apr 13, 2023

Merge pull request #48809 from ven-k/vkb/imageing-default

9aa7669

Fix imaging value for generating sysimages for multiple targets (cherry picked from commit fc4b079)

ven-k mentioned this pull request Apr 19, 2023

Pass CPU targets as cmdline args JuliaLang/PackageCompiler.jl#804

Merged

KristofferC removed the backport 1.9 Change should be backported to release-1.9 label Apr 25, 2023

vchuravy mentioned this pull request Oct 26, 2023

Respect JULIA_CPU_TARGET set in Make.inc when building pkgimgs #51886

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix imaging value for generating sysimages for multiple targets #48809

Fix imaging value for generating sysimages for multiple targets #48809

ven-k commented Feb 27, 2023

vchuravy commented Feb 27, 2023

vchuravy Feb 27, 2023

vchuravy Feb 27, 2023

vtjnash Feb 27, 2023

vtjnash Feb 27, 2023

ven-k Feb 28, 2023

ven-k Feb 28, 2023 •

edited

Loading

vtjnash Feb 28, 2023

DilumAluthge commented Feb 28, 2023

DilumAluthge commented Feb 28, 2023

DilumAluthge commented Mar 4, 2023

vchuravy commented Mar 5, 2023 •

edited

Loading

DilumAluthge commented Mar 30, 2023

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

vchuravy commented Mar 31, 2023

staticfloat commented Apr 11, 2023

vchuravy left a comment

staticfloat commented Apr 12, 2023

ven-k commented Apr 12, 2023

DilumAluthge commented Apr 12, 2023

	--cpu-target='generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)'
	--cpu-target='generic;generic'

Fix imaging value for generating sysimages for multiple targets #48809

Fix imaging value for generating sysimages for multiple targets #48809

Conversation

ven-k commented Feb 27, 2023

Current issue:

After this PR, these work:

vchuravy commented Feb 27, 2023

vchuravy Feb 27, 2023

Choose a reason for hiding this comment

vchuravy Feb 27, 2023

Choose a reason for hiding this comment

vtjnash Feb 27, 2023

Choose a reason for hiding this comment

vtjnash Feb 27, 2023

Choose a reason for hiding this comment

ven-k Feb 28, 2023

Choose a reason for hiding this comment

ven-k Feb 28, 2023 • edited Loading

Choose a reason for hiding this comment

vtjnash Feb 28, 2023

Choose a reason for hiding this comment

DilumAluthge commented Feb 28, 2023

DilumAluthge commented Feb 28, 2023

DilumAluthge commented Mar 4, 2023

vchuravy commented Mar 5, 2023 • edited Loading

DilumAluthge commented Mar 30, 2023

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

vchuravy commented Mar 31, 2023

staticfloat commented Apr 11, 2023

vchuravy left a comment

Choose a reason for hiding this comment

staticfloat commented Apr 12, 2023

ven-k commented Apr 12, 2023

DilumAluthge commented Apr 12, 2023

ven-k Feb 28, 2023 •

edited

Loading

vchuravy commented Mar 5, 2023 •

edited

Loading