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

Default to the medium code model in x86 linux #53391

Merged
merged 6 commits into from
Mar 14, 2024
Merged

Default to the medium code model in x86 linux #53391

merged 6 commits into from
Mar 14, 2024

Conversation

gbaraldi
Copy link
Member

@gbaraldi gbaraldi commented Feb 19, 2024

This shouldn't have any cost on smaller images because the only thing that gets put into ldata is the system image data, which is only reference via dlsym. This allows for images larger than 2gb (tested by putting a 2gb array in the base image)

I did not test how this might be handled in other platforms (windows doesn't support it)

We may want to backport to 1.10/1.11

@gbaraldi gbaraldi requested review from vchuravy and vtjnash February 19, 2024 19:48
Comment on lines +1660 to +1661
if (TheTriple.isX86() && TheTriple.isArch64Bit() && TheTriple.isOSLinux())
sysdata->setSection(".ldata");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, this is already automatic, once you select Medium code model globally, although can also be opted for explicitly for each variable:
https://llvm.org/doxygen/TargetMachine_8cpp_source.html

Suggested change
if (TheTriple.isX86() && TheTriple.isArch64Bit() && TheTriple.isOSLinux())
sysdata->setSection(".ldata");
sysdata->setCodeModel(CodeModel::Large);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't doing it automatically for me :(. Not sure why

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

llvm/llvm-project#72078 also the per global option was just added.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this should have an LLVM_VERSION ifdef here then?

@vtjnash vtjnash added compiler:codegen Generation of LLVM IR and native code compiler:precompilation Precompilation of modules backport 1.11 Change should be backported to release-1.11 labels Feb 19, 2024
@KristofferC KristofferC mentioned this pull request Feb 26, 2024
28 tasks
KristofferC added a commit that referenced this pull request Mar 1, 2024
Backported PRs:
- [x] #53361 <!-- 🤖 [master] Bump the SparseArrays stdlib from c9f7293
to cb602d7 -->
- [x] #53300 <!-- allow external absint to hold custom data in
`codeinst.inferred` -->
- [x] #53342 <!-- Add `Base.wrap` to docs -->
- [x] #53372 <!-- Silence warnings in `test/file.jl` -->
- [x] #53357 <!-- 🤖 [master] Bump the Pkg stdlib from 6dd0e7c9e to
76070d295 -->
- [x] #53373 <!-- fix sysimage-native-code=no option with pkgimages -->
- [x] #53333 <!-- More consistent return value for annotations, take 2
-->
- [x] #53354 <!-- fix code coverage bug in tail position and `else` -->
- [x] #53407 <!-- fix sysimage-native-code=yes option -->
- [x] #53388 <!-- Fix documentation: thread pool of main thread -->
- [x] #53355 <!-- Fix synchronization issues on the GC scheduler. -->
- [x] #53429 <!-- Subtype: skip slow-path in `local_∀_∃_subtype` if
inputs contain no ∃ typevar. -->
- [x] #53437 <!-- Add debug variant of loader_trampolines.o -->
- [x] #53284 <!-- Add annotate! method for AnnotatedIOBuffer -->
- [x] #53466 <!-- [MozillaCACerts_jll] Update to v2023-12-12 -->
- [x] #53467 <!-- [LibGit2_jll] Update to v1.7.2 -->
- [x] #53326 <!-- RFC: when loading code for internal purposes, load
stdlib files directly, bypassing DEPOT_PATH, LOAD_PATH, and stale checks
-->
- [x] #53332
- [x] #53320 <!-- Add `Sys.isreadable, Sys.iswriteable`, update `ispath`
-->
- [x] #53476

Contains multiple commits, manual intervention needed:
- [ ] #53285 <!-- Add update mechanism for Terminfo, and common
user-alias data -->

Non-merged PRs with backport label:
- [ ] #53424 <!-- yet more atomics & cache-line fixes on work-stealing
queue -->
- [ ] #53408 <!-- task splitting: change additive accumulation to
multiplicative -->
- [ ] #53403 <!-- Move parallel precompilation to Base -->
- [ ] #53402 <!-- Add `jl_getaffinity` and `jl_setaffinity` -->
- [ ] #53391 <!-- Default to the medium code model in x86 linux -->
- [ ] #53125 <!-- coverage: count coverage where explicitly requested by
inference only -->
- [ ] #52694 <!-- Reinstate similar for AbstractQ for backward
compatibility -->
@KristofferC KristofferC mentioned this pull request Mar 1, 2024
60 tasks
@ven-k
Copy link
Member

ven-k commented Mar 4, 2024

Can this be backported to 1.10?

@gbaraldi gbaraldi added the backport 1.10 Change should be backported to the 1.10 release label Mar 4, 2024
@KristofferC KristofferC mentioned this pull request Mar 12, 2024
25 tasks
src/aotcompile.cpp Outdated Show resolved Hide resolved
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Mar 14, 2024
@vchuravy vchuravy removed their request for review March 14, 2024 15:56
@vtjnash vtjnash merged commit 0f04b33 into master Mar 14, 2024
5 of 8 checks passed
@vtjnash vtjnash deleted the gb/large-image branch March 14, 2024 19:30
KristofferC pushed a commit that referenced this pull request Mar 15, 2024
This shouldn't have any cost on smaller images because the only thing
that gets put into ldata is the system image data, which is only
reference via `dlsym`. This allows for images larger than 2gb (tested by
putting a 2gb array in the base image)

I did not test how this might be handled in other platforms (Windows
doesn't support it).

(cherry picked from commit 0f04b33)
KristofferC added a commit that referenced this pull request Mar 17, 2024
Backported PRs:
- [x] #39071 <!-- Add a lazy `logrange` function and `LogRange` type -->
- [x] #51802 <!-- Allow AnnotatedStrings in log messages -->
- [x] #53369 <!-- Orthogonalize re-indexing for FastSubArrays -->
- [x] #48050 <!-- improve `--heap-size-hint` arg handling -->
- [x] #53482 <!-- add IR encoding for EnterNode -->
- [x] #53499 <!-- Avoid compiler warning about redefining jl_globalref_t
-->
- [x] #53507 <!-- update staled `Core.Compiler.Effects` documentation
-->
- [x] #53408 <!-- task splitting: change additive accumulation to
multiplicative -->
- [x] #53523 <!-- add back an alias for `check_top_bit` -->
- [x] #53377 <!-- add _readdirx for returning more object info gathered
during dir scan -->
- [x] #53525 <!-- fix InteractiveUtils call in Base.runtests on failure
-->
- [x] #53540 <!-- use more efficient `_readdirx` for tab completion -->
- [x] #53545 <!-- use `_readdirx` for `walkdir` -->
- [x] #53551 <!-- revert "Add @create_log_macro for making custom styled
logging macros (#52196)" -->
- [x] #53554 <!-- Always return a value in 1-d circshift! of
abstractarray.jl -->
- [x] #53424 <!-- yet more atomics & cache-line fixes on work-stealing
queue -->
- [x] #53571 <!-- Update Documenter to v1.3 for inventory writing -->
- [x] #53403 <!-- Move parallel precompilation to Base -->
- [x] #53589 <!-- add back `unsafe_convert` to pointer for arrays -->
- [x] #53596 <!-- build: remove extra .a file -->
- [x] #53606 <!-- fix error path in `precompilepkgs` -->
- [x] #53004 <!-- Unexport with, at_with, and ScopedValue from Base -->
- [x] #53629 <!-- typo fix in scoped values docs -->
- [x] #53630 <!-- sroa: Fix incorrect scope counting -->
- [x] #53598 <!-- Use Base parallel precompilation to build stdlibs -->
- [x] #53649 <!-- precompilepkgs: package in boths deps and weakdeps are
in fact only weak -->
- [x] #53671 <!-- Fix bootstrap Base precompile in cross compile
configuration -->
- [x] #52125 <!-- Load Pkg if not already to reinstate missing package
add prompt -->
- [x] #53602 <!-- Handle zero on arrays of unions of number types and
missings -->
- [x] #53516 <!-- permit NamedTuple{<:Any, Union{}} to be created -->
- [x] #53643 <!-- Bump CSL to 1.1.1 to fix libgomp bug -->
- [x] #53679 <!-- move precompile workload back from Base -->
- [x] #53663 <!-- add isassigned methods for reinterpretarray -->
- [x] #53662 <!-- [REPL] fix incorrectly cleared line after completions
accepted -->
- [x] #53611 <!-- Linalg: matprod_dest for Diagonal and adjvec -->
- [x] #53659 <!-- fix #52025, re-allow all implicit pointer casts in
cconvert for Array -->
- [x] #53631 <!-- LAPACK: validate input parameters to throw informative
errors -->
- [x] #53628 <!-- Make some improvements to the Scoped Values
documentation. -->
- [x] #53655 <!-- Change tbaa of ptr_phi to tbaa_value  -->
- [x] #53391 <!-- Default to the medium code model in x86 linux -->
- [x] #53699 <!-- Move `isexecutable, isreadable, iswritable` to
`filesystem.jl` -->
- [x] #41232 <!-- Fix linear indexing for ReshapedArray if the parent
has offset axes -->
- [x] #53527 <!-- Enable analyzegc checks for try catch and fix found
issues -->
- [x] #52092 
- [x] #53682 <!-- Increase build precompilation -->
- [x] #53720 
- [x] #53553 <!-- typeintersect: fix `UnionAll` unaliasing bug caused by
innervars. -->

Contains multiple commits, manual intervention needed:
- [ ] #53305 <!-- Propagate inbounds in isassigned with CartesianIndex
indices -->

Non-merged PRs with backport label:
- [ ] #53736 <!-- fix literal-pow to return the right type when the base
is -1 -->
- [ ] #53707 <!-- Make ScopedValue public -->
- [ ] #53696 <!-- add invokelatest to on_done callback in bracketed
paste -->
- [ ] #53660 <!-- put Logging back in default sysimage -->
- [ ] #53509 <!-- revert moving "creating packages" from Pkg.jl -->
- [ ] #53452 <!-- RFC: allow Tuple{Union{}}, returning Union{} -->
- [ ] #53402 <!-- Add `jl_getaffinity` and `jl_setaffinity` -->
- [ ] #52694 <!-- Reinstate similar for AbstractQ for backward
compatibility -->
- [ ] #51928 <!-- Styled markdown, with a few tweaks -->
- [ ] #51816 <!-- User-themable stacktraces -->
- [ ] #51811 <!-- Make banner size depend on terminal size -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label Mar 18, 2024
KristofferC pushed a commit that referenced this pull request Mar 18, 2024
This shouldn't have any cost on smaller images because the only thing
that gets put into ldata is the system image data, which is only
reference via `dlsym`. This allows for images larger than 2gb (tested by
putting a 2gb array in the base image)

I did not test how this might be handled in other platforms (Windows
doesn't support it).

(cherry picked from commit 0f04b33)
KristofferC pushed a commit that referenced this pull request Mar 18, 2024
This shouldn't have any cost on smaller images because the only thing
that gets put into ldata is the system image data, which is only
reference via `dlsym`. This allows for images larger than 2gb (tested by
putting a 2gb array in the base image)

I did not test how this might be handled in other platforms (Windows
doesn't support it).

(cherry picked from commit 0f04b33)
topolarity pushed a commit that referenced this pull request Mar 25, 2024
This shouldn't have any cost on smaller images because the only thing
that gets put into ldata is the system image data, which is only
reference via `dlsym`. This allows for images larger than 2gb (tested by
putting a 2gb array in the base image)

I did not test how this might be handled in other platforms (Windows
doesn't support it).

(cherry picked from commit 0f04b33)
@IanButterworth IanButterworth removed the merge me PR is reviewed. Merge when all tests are passing label Apr 5, 2024
KristofferC added a commit that referenced this pull request Apr 16, 2024
@maleadt
Copy link
Member

maleadt commented Apr 16, 2024

This PR breaks precompilation packages like LLVM, when applying this change to LLVM 15 from Julia 1.10:

Precompiling project...
lld: error: relocation R_X86_64_64 cannot be used against symbol 'jlcapi_handle_error_4257'; recompile with -fPIC
>>> defined in /home/tim/.julia/compiled/v1.10/LLVM/jl_bPThkr(text#1.o)
>>> referenced by libLLVM.jl:232 (/home/tim/.julia/packages/LLVM/bzSzE/lib/15/libLLVM.jl:232)
>>>               text#4.o:(julia___init___4235) in archive /home/tim/.julia/compiled/v1.10/LLVM/jl_bPThkr

lld: error: relocation R_X86_64_64 cannot be used against symbol 'jlcapi_handle_diagnostic_4271'; recompile with -fPIC
>>> defined in /home/tim/.julia/compiled/v1.10/LLVM/jl_bPThkr(text#5.o)
>>> referenced by libLLVM.jl:580 (/home/tim/.julia/packages/LLVM/bzSzE/lib/15/libLLVM.jl:580)
>>>               text#4.o:(julia___init___4235) in archive /home/tim/.julia/compiled/v1.10/LLVM/jl_bPThkr

The functions mentioned here all define @cfunction callbacks.

MWE:

module Foo
register() = @cfunction(identity, Cint, (Cint,))
precompile(register, ())
end
❯ JULIA_LOAD_PATH=/tmp ./julia -e 'using Foo'
lld: error: relocation R_X86_64_64 cannot be used against local symbol; recompile with -fPIC
>>> defined in /home/tim/.julia/compiled/v1.10/jl_fshGei(text#0.o)
>>> referenced by Foo.jl:2 (/tmp/Foo.jl:2)
>>>               text#0.o:(julia_register_52) in archive /home/tim/.julia/compiled/v1.10/jl_fshGei

The above produces the following archive: input.zip

lld from LLVM 16 isn't able to link this archive either, so it's probably related to the archive rather than the linker:

❯ LD_LIBRARY_PATH=/home/tim/Julia/depot/juliaup/julia-nightly/lib/julia /home/tim/Julia/depot/juliaup/julia-nightly/libexec/julia/lld -flavor gnu '' -shared --whole-archive /tmp/input.a --no-whole-archive -L/home/tim/Julia/src/julia/build/dev/usr/lib/julia -L/home/tim/Julia/src/julia/build/dev/usr/lib -ljulia -ljulia-internal
lld: error: relocation R_X86_64_64 cannot be used against local symbol; recompile with -fPIC
>>> defined in /tmp/input.a(text#0.o)
>>> referenced by Foo.jl:2 (/tmp/Foo.jl:2)
>>>               text#0.o:(julia_register_52) in archive /tmp/input.a

KristofferC added a commit that referenced this pull request Apr 22, 2024
Backported PRs:
- [x] #50759 <!-- Fix outdated usage of scrubbing for log test failures
-->
- [x] #51830 <!-- Add version string to sysimg triple -->
- [x] #53273 <!-- [REPL] Fix typo in using/import completion -->
- [x] #53499 <!-- Avoid compiler warning about redefining jl_globalref_t
-->
- [x] #53424 <!-- yet more atomics & cache-line fixes on work-stealing
queue -->
- [x] #53596 <!-- build: remove extra .a file -->
- [x] #53516 <!-- permit NamedTuple{<:Any, Union{}} to be created -->
- [x] #53643 <!-- Bump CSL to 1.1.1 to fix libgomp bug -->
- [x] #53655 <!-- Change tbaa of ptr_phi to tbaa_value  -->
- [x] #53391 <!-- Default to the medium code model in x86 linux -->
- [x] #53809 <!-- Add missing GC_POP() in emit_cfunction -->
- [x] #53961 <!-- `LazyString` in `LinearAlgebra.checksquare` error
message -->
- [x] #52913 <!-- Added docstring for Artifacts.jl -->
- [x] #53553 <!-- typeintersect: fix `UnionAll` unaliasing bug caused by
KristofferC pushed a commit that referenced this pull request May 8, 2024
This shouldn't have any cost on smaller images because the only thing
that gets put into ldata is the system image data, which is only
reference via `dlsym`. This allows for images larger than 2gb (tested by
putting a 2gb array in the base image)

I did not test how this might be handled in other platforms (Windows
doesn't support it).

(cherry picked from commit 0f04b33)
@KristofferC KristofferC mentioned this pull request May 8, 2024
23 tasks
@KristofferC KristofferC removed the backport 1.10 Change should be backported to the 1.10 release label May 8, 2024
gbaraldi added a commit that referenced this pull request May 15, 2024
DelveCI pushed a commit to RelationalAI/julia that referenced this pull request May 26, 2024
This shouldn't have any cost on smaller images because the only thing
that gets put into ldata is the system image data, which is only
reference via `dlsym`. This allows for images larger than 2gb (tested by
putting a 2gb array in the base image)

I did not test how this might be handled in other platforms (Windows
doesn't support it).

(cherry picked from commit 0f04b33)
DelveCI pushed a commit to RelationalAI/julia that referenced this pull request May 28, 2024
This shouldn't have any cost on smaller images because the only thing
that gets put into ldata is the system image data, which is only
reference via `dlsym`. This allows for images larger than 2gb (tested by
putting a 2gb array in the base image)

I did not test how this might be handled in other platforms (Windows
doesn't support it).

(cherry picked from commit 0f04b33)
DelveCI pushed a commit to RelationalAI/julia that referenced this pull request May 29, 2024
This shouldn't have any cost on smaller images because the only thing
that gets put into ldata is the system image data, which is only
reference via `dlsym`. This allows for images larger than 2gb (tested by
putting a 2gb array in the base image)

I did not test how this might be handled in other platforms (Windows
doesn't support it).

(cherry picked from commit 0f04b33)
Drvi pushed a commit to RelationalAI/julia that referenced this pull request Jun 7, 2024
This shouldn't have any cost on smaller images because the only thing
that gets put into ldata is the system image data, which is only
reference via `dlsym`. This allows for images larger than 2gb (tested by
putting a 2gb array in the base image)

I did not test how this might be handled in other platforms (Windows
doesn't support it).

(cherry picked from commit 0f04b33)
Drvi pushed a commit to RelationalAI/julia that referenced this pull request Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code compiler:precompilation Precompilation of modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants