Actually setup jit targets when compiling packageimages instead of targeting only one

[gbaraldi]

I haven't assembled a test for this just yet, but this works
Fixes #54464

[vchuravy]

I think we need to test that we still pick the right/one for all.

I have a unsubstantiated fear about loading images with mismatched ABI. Previously this code said to pick one and then ensured that everyone is consistent.

[gbaraldi]

So what guarantees the right one is sysimg_init_cb. We unconditionally look for the first one.

[vchuravy]

So checking that my understanding is correct. The issue is that when during sysimg creation we don't yet have a native cache so we are not filling jit targets based on that. Instead we actually parse the -C string.

But for pkgimages we have an image loaded, so ensure_jit_targets early exists and so the multi versioning infrastructure doesn't see the rest?

Maybe we should split parsing for output and choosing for loading/jit?

[gbaraldi]

So the code in ensure_jit_targets only seems to actually do anything when building an image, so maybe we should move it around?

[giordano]

With the script at #54464 (comment) I get

% JULIA_CPU_TARGET='sandybridge,-xsaveopt;generic,clone_all;haswell,-rdrnd,base(1);x86-64-v4,-rdrnd,base(1)' julia-master example_targets.jl 
ERROR: LoadError: Base.PkgId(Base.UUID("7876af07-990d-54b4-ab0e-23690620f79a"), "Example") has not yet been precompiled for julia 1.12.0-DEV.531
Stacktrace:
 [1] error(::Base.PkgId, ::String, ::VersionNumber)
   @ Base ./error.jl:53
 [2] top-level scope
   @ /tmp/example_targets.jl:13
 [3] include
   @ ./Base.jl:559 [inlined]
 [4] exec_options(opts::Base.JLOptions)
   @ Base ./client.jl:325
 [5] _start()
   @ Base ./client.jl:533
in expression starting at /tmp/example_targets.jl:13

The .julia/compiled/v1.12/Example directory is empty. Without JULIA_CPU_TARGET:

% julia-master example_targets.jl                  
targets = Base.ImageTarget[haswell; flags=0; features_en=(sse3, pclmul, ssse3, fma, cx16, sse4.1, sse4.2, movbe, popcnt, aes, xsave, avx, f16c, fsgsbase, bmi, avx2, bmi2, sahf, lzcnt), haswell; flags=0; features_en=(sse3, pclmul, ssse3, fma, cx16, sse4.1, sse4.2, movbe, popcnt, aes, xsave, avx, f16c, fsgsbase, bmi, avx2, bmi2, sahf, lzcnt)]

Note that the target haswell is repeated twice.

[gbaraldi]

This isn't working :(

[giordano]

Small progress! Without JULIA_CPU_TARGET:

% julia-master example_targets.jl                     
targets = Base.ImageTarget[haswell; flags=0; features_en=(sse3, pclmul, ssse3, fma, cx16, sse4.1, sse4.2, movbe, popcnt, aes, xsave, avx, f16c, fsgsbase, bmi, avx2, bmi2, sahf, lzcnt)]

Now there's a single target, rather than two. But whenever JULIA_CPU_TARGET is set:

% JULIA_CPU_TARGET='generic' julia-master example_targets.jl              
ERROR: LoadError: Base.PkgId(Base.UUID("7876af07-990d-54b4-ab0e-23690620f79a"), "Example") has not yet been precompiled for julia 1.12.0-DEV.531
Stacktrace:
 [1] error(::Base.PkgId, ::String, ::VersionNumber)
   @ Base ./error.jl:53
 [2] top-level scope
   @ /tmp/example_targets.jl:13
 [3] include
   @ ./Base.jl:559 [inlined]
 [4] exec_options(opts::Base.JLOptions)
   @ Base ./client.jl:325
 [5] _start()
   @ Base ./client.jl:533
in expression starting at /tmp/example_targets.jl:13

[IanButterworth]

Recording here so this isn't lost on slack.

There's a ~27% increase in pkgimage size with this PR.

master (via juliaup)

julia> using PkgCacheInspector 

julia> info_cachefile("Pkg")
Contents of /Users/ian/.julia/juliaup/julia-nightly/share/julia/compiled/v1.12/Pkg/tUTdb_myMUk.dylib:
  modules: Any[Pkg.MiniProgressBars, Pkg.GitTools, Pkg.PlatformEngines, Pkg.Versions, Pkg.Registry, Pkg.Resolve, Pkg.Types.FuzzySorting, Pkg.Types, Pkg.BinaryPlatforms, Pkg.Artifacts, Pkg.Operations, Pkg.API, Pkg.REPLMode, Pkg]
  init order: Any[Pkg]
  529 external methods
  16850 new specializations of external methods (Base 79.9%, Base.Broadcast 7.6%, Base.Sort 3.7%, ...)
  1585 external methods with new roots
  33049 external targets
  24615 edges
  file size:   47139456 (44.956 MiB)
  Segment sizes (bytes):
    system:      19914892 ( 47.10%)
    isbits:      19778248 ( 46.78%)
    symbols:        80497 (  0.19%)
    tags:          336949 (  0.80%)
    relocations:  2082887 (  4.93%)
    gvars:          41200 (  0.10%)
    fptrs:          44632 (  0.11%)
  Image targets: 
    generic; flags=0; features_en=()

This PR built locally on MacOS with the same buildkite targets

JULIA_CPU_TARGET=generic;cortex-a57;thunderx2t99;carmel,clone_all;apple-m1,base(3);neoverse-512tvb,base(3)

julia> info_cachefile("Pkg")
Contents of /Users/ian/Documents/GitHub/julia/usr/share/julia/compiled/v1.12/Pkg/tUTdb_p5Nph.dylib:
  modules: Any[Pkg.MiniProgressBars, Pkg.GitTools, Pkg.PlatformEngines, Pkg.Versions, Pkg.Registry, Pkg.Resolve, Pkg.Types.FuzzySorting, Pkg.Types, Pkg.BinaryPlatforms, Pkg.Artifacts, Pkg.Operations, Pkg.API, Pkg.REPLMode, Pkg]
  init order: Any[Pkg]
  529 external methods
  16819 new specializations of external methods (Base 79.9%, Base.Broadcast 7.6%, Base.Sort 3.7%, ...)
  1581 external methods with new roots
  32968 external targets
  24583 edges
  file size:   59591872 (56.831 MiB)
  Segment sizes (bytes):
    system:      19918588 ( 47.19%)
    isbits:      19712700 ( 46.70%)
    symbols:        76445 (  0.18%)
    tags:          336483 (  0.80%)
    relocations:  2080035 (  4.93%)
    gvars:          41216 (  0.10%)
    fptrs:          44520 (  0.11%)
  Image targets: 
    generic; flags=0; features_en=()
    cortex-a57; flags=0; features_en=(crc)
    thunderx2t99; flags=0; features_en=(aes, sha2, crc, lse, rdm, v8_1a)
    carmel; flags=0; features_en=(aes, sha2, crc, lse, fullfp16, rdm, ccpp, v8_1a, v8_2a)
    apple-m1; flags=0; features_en=(aes, sha2, crc, lse, fullfp16, rdm, jsconv, complxnum, rcpc, ccpp, sha3, dotprod, fp16fml, dit, rcpc-immo, flagm, sb, ccdp, altnzcv, fptoint, v8_1a, v8_2a, v8_3a, v8_4a, v8_5a)
    neoverse-512tvb; flags=32; features_en=()

[StefanKarpinski]

That's not bad for being having efficient code for multiple platforms! This is definitely something we'll want for serving pkgimgs.

[gbaraldi]

There is a windows issue somehow :\

[IanButterworth]

If there's no clear reason for the windows issue is it worth disabling on windows and merging this to get it tested?

[gbaraldi]

I'm not super confortable merging it because that windows issue doesn't seem to be simple.

[giordano]

What's the status of this PR? I had a look a few days ago, my understanding is that a local native build of Julia which doesn't set JULIA_CPU_TARGET during the compilation of the sysimage can't generate pkgimages for different (or smaller than "native" for the host?) ISAs, while setting JULIA_CPU_TARGET during sysimage building allows compiling pkgimages for targets compatible with the those included in the sysimage. Is this an accurate description? If so, I feel like this should be better documented, because it isn't very obvious.

[gbaraldi]

Yes, basically pkgimages can only be more specific than the base system image so if the the base sysimage is native, then the pkgimages are also native.

[Octogonapus]

Great to have this improved but I agree with Mose that this is not clear:

basically pkgimages can only be more specific than the base system image

Is an error returned if the user tries to make a package image for a totally different target than the base image? I don't fully understand this code but it seems that there is no check for that case. Would it be feasible to implement such a check? Otherwise this feels like a footgun. I could also update the docs here and here but I don't think that would be enough.

[giordano]

@KristofferC why did you remove the backport-1.11 label without a comment? There's only a merge conflict in the tests, but it's trivial to fix it. I'm happy to push it to #56228 if that's ok for you.

[KristofferC]

Sorry, I should have written something. It was discussed that this was too big of a thing to backport as late as it was in the release cycle. Now that it has been on master for a while, perhaps that can be reconsidered.

[giordano]

Since backporting this PR would fix #56177, I'm inclined to do it.