- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Backports release 1.12 #57536
Open
KristofferC
wants to merge
68
commits into
release-1.12
Choose a base branch
from
backports-release-1.12
base: release-1.12
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Backports release 1.12 #57536
+3,456
−1,955
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ional` (#55853) Relying on `ScopedValues`, set `BigFloat` precision without mutating the global default, while constructing `Rational` from `AbstractIrrational`. Also helps avoid reading the global defaults for the precision and rounding mode, together with #56095. What does this fix: * in the case of the `Irrational` constants defined in `MathConstants`: relevant methods have `@assume_effects :foldable` applied, which includes `:effect_free`, which requires that no globals be mutated (followup on #55886) * in the case of `AbstractIrrational` values in general, this PR prevents data races on the global `BigFloat` precision (cherry picked from commit 2a89f71)
Reimplement all of the trim verification support in Julia as a compiler analysis pass. Move all this verification code into the Compiler julia code, where we have much better utilities for pretty printing and better observability for analysis. (cherry picked from commit 1045bd8)
Recompute some O(n) things a bit less on every statement. Fix an assert that ensured LimitedAccuracy does not accidentally get preserved when it should have been deleted: the (local) cache should not contain things that are marked as dead (RIP), as that was leading to much code not getting cached when it logically should have. Simplify the computation of frame_parent when the cycle_parent isn't needed. (cherry picked from commit 0366a2a)
- disable load path setup - remove upstreamed change to utf8proc_map - fix warning for accessing mod.main in the wrong world - remove binding backedges when trimming (cherry picked from commit d77c24f)
This change removes an inconsistency between `>:` and `<:`. We were parsing `where {A>:B>:C}` forms, but not recognizing them in lowering. Before: ``` julia> Vector{T} where Int<:T<:Number Vector{T} where Int64<:T<:Number julia> Vector{T} where Number>:T>:Int ERROR: syntax: invalid bounds in "where" around REPL[14]:1 ``` After: ``` julia> Vector{T} where Number>:T>:Int Vector{T} where Int64<:T<:Number ``` (cherry picked from commit 2e57730)
This restores 1.11 behavior. However, I find this function a bit problematic, since it is asking whether the binding is `constant` in a particular sense, but not whether it is `const` (in the sense of having been declared with the keyword or equivalent), so the shortening is confusing. However, we don't need to address that now. Fixes #57475. (cherry picked from commit 1ff98a5)
We have historically allowed the following without warning or error: ``` const x = 1 x = 1 ``` As well as ``` const x = 1 x = 2 ``` with a UB warning. In 1.12, we made the second case error, as part of the general binding partition changes, but did not touch the first case, because it did not have the warning. I think this made a reasonable amount of sense, because we essentially treated constants as special kinds of mutable globals (to which assignment happened to be UB). However, in the 1.12+ design, constants and globals are quite sepearate beasts, and I think it makes sense to extend the error to the egal case also, even if it is technically more breaking. In fact, I already thought that's what I did when I implemented the new effect model for global assignment, causing #57566. I can't think of a legitimate reason to keep this special case. For those who want to do binding replacement, the `const` keyword is mandatory, so the assignment is now literally always a semantic no-op or an error. Fixes #57566. (cherry picked from commit 8f00a51)
This is my attempt to resolve #53000. ``` julia> solve ERROR: UndefVarError: `solve` not defined in `Main` Suggestion: check for spelling errors or missing imports. Hint: a global variable of this name also exists in CommonSolve. - Also exported by SciMLBase. - Also exported by DiffEqBase. - Also exported by JumpProcesses. - Also exported by LinearSolve. - Also exported by BracketingNonlinearSolve (loaded but not imported in Main). - Also exported by SimpleNonlinearSolve. - Also exported by NonlinearSolve. - Also exported by OrdinaryDiffEqLowStorageRK (loaded but not imported in Main). - Also exported by OrdinaryDiffEqSSPRK (loaded but not imported in Main). - Also exported by OrdinaryDiffEqVerner (loaded but not imported in Main). - Also exported by OrdinaryDiffEqBDF (loaded but not imported in Main). - Also exported by OrdinaryDiffEqTsit5 (loaded but not imported in Main). - Also exported by OrdinaryDiffEqRosenbrock (loaded but not imported in Main). - Also exported by OrdinaryDiffEqDefault (loaded but not imported in Main). - Also exported by Sundials. ``` I would have beefed up the test case, but can't follow how it works. --------- Co-authored-by: Alex Arslan <ararslan@comcast.net> Co-authored-by: Ian Butterworth <i.r.butterworth@gmail.com> Co-authored-by: Jameson Nash <vtjnash@gmail.com> (cherry picked from commit 5f13cd2)
These were the intended semantics of #57311 (and matches what it used to do in 1.11). Note however that this differs from the body-ful form, which now always tries to extend. Fixes #57546. Note that this implementation is slightly inefficient since it goes through a binding replacement. However, there's a change coming down the line which optimizes these replacements. I do think it should eventually be refactored to just create the binding directly, but that's a little bit of a larger change. (cherry picked from commit 7fa0c13)
…gler (#57579) In the line of C code: ```C const int64_t timeout = jl_options.timeout_for_safepoint_straggler_s * 1000000000; ``` `jl_options.timeout_for_safepoint_straggler_s` is an `int16_t` and `1000000000` is an `int32_t`. The result of `jl_options.timeout_for_safepoint_straggler_s * 1000000000` will be an `int32_t` which may not be large enough to hold the value of `jl_options.timeout_for_safepoint_straggler_s` after converting to nanoseconds, leading to overflow. (cherry picked from commit 9cafd35)
The `_apply_pure` function only has one user, and that user is unsound anyways and should not be used, so replace that with equivalent `_call_in_world_total` call and remove unnecessary definitions. The awkward distinction between `invokelatest` and `_call_latest` has not been relevant (and indeed causes performance issues) since kwfunc was introduced in #47157. (cherry picked from commit 671c6a1)
--------- Co-authored-by: Kristoffer <kcarlsson89@gmail.com> Co-authored-by: Ian Butterworth <i.r.butterworth@gmail.com> (cherry picked from commit 0dd78f7)
This repeats the exercise in #57405. This is required for correctness, because the ->deprecated flag also affects `using` resolution (since it makes the tagged binding weaker for `using` purposes). That said, in general our `->deprecated` semantics have been somewhat underspecified with lots of `XXX` comments in the surrounding code. This tries to define the semantics to give a depwarn on *access* (read or write) when: 1. Either the binding itself is deprecated; or 2. The implicit imports pass through a deprecated binding. However, we do not give depwarns on access to bindings that were explicitly imported (although we do give those warnings on the import) - the reasoning being that it's the import that needs to be adjusted not the access. Additionally, this PR moves into the direction of making the depwarn a semantic part of the global access, by adjusting codegen and inference appropriately. (cherry picked from commit 1a3cbb1)
When inference and codegen queries the partitions of a binding, they often only care about a subset of the information in the partition. At the moment, we always truncate world ranges to the most precise partition that contains the relevant query. However, we can instead expand the world range to cover all partitions that are equivalent for the given query. To give a concrete example, both inference and codegen never care about the exported flag in a binding partition, so we should not unnecessarily truncate a world range just because an export was introduced. Further, this commit lays the ground work to stop invalidating code for these same kinds of transitions, although the actual logic to do that will come in a separate PR. (cherry picked from commit a5157c0)
This renames the partition flags to start with PARTITION_ and turns the Binding flags into a bitfield that can be accessed atomically in preparation for adding further flags. (cherry picked from commit a802faf)
Our implicit edge tracking for bindings does not explicitly store any edges for bindings in the *current* module. The idea behind this is that this is a good time-space tradeoff for validation, because substantially all binding references in a module will be to its defining module, while the total number of methods within a module is limited and substantially smaller than the total number of methods in the entire system. However, we have an issue where the code that stores these edges and the invalidation code disagree on which module is the *current* one. The edge storing code was using the module in which the method was defined, while the invalidation code was using the one in which the MethodTable is defined. With these being misaligned, we can miss necessary invalidations. Both options are in principle possible, but I think the former is better, because the module in which the method is defined is also the module that we are likely to have a lot of references to (since they get referenced implicitly by just writing symbols in the code). However, this presents a problem: We don't actually have a way to iterate all the methods defined in a particular module, without just doing the brute force thing of scanning all methods and filtering. To address this, build on the deferred scanning code added in #57615 to also add any scanned modules to an explicit list in `Module`. This costs some space, but only proportional to the number of defined methods, (and thus proportional to the written source code). Note that we don't actually observe any issues in the test suite on master due to this bug. However, this is because we are grossly over-invalidating, which hides the missing invalidations from this issue (#57617). (cherry picked from commit 274d80e)
@nanosoldier |
1.12 branch version of #57679
Make this function `--trim` compatible. (cherry picked from commit 8edc2b3)
The alignment of a nested object (in C layouts) is not affected by the alignment of its parent container when computing a field offset (as if it will be allocated at address 0). This can be strongly counter-intuitive (as it implies it should add padding where it does not seem to provide value to the user), but this is required to match the C standard. It also permits users to write custom allocators which happen to provide alignment in excess of that which codegen may assume is guaranteed, and get the behavioral characteristics they intended to specify (without resorting to computing explicit padding). Addresses #57713 (comment) (cherry picked from commit c9008ff)
This makes `LazyLibraryPath` eligible for const-prop, which is important so that we can inline/infer `string(::LazyLibraryPath)` at compile-time if the argument is a (global) `const`. Otherwise, this code is not infer-able without something like TypedCallable to make the interface requirements explicit. (cherry picked from commit 3f4eda6)
Should get rid of some invalidation on running user code. (cherry picked from commit d96a6e6)
…ed variables (#57576) For some reason, the nice error hints for undefined variables are put in the REPL module, requiring it to be loaded to get good error messages. There are many cases where code runs outside of the REPL but where good error messages are still useful (PkgEval, `Pkg.test`, Pluto, IJulia). As an example, this is what I got when running `Pkg.test` for a package with a regression: ``` Testing Running tests... ERROR: LoadError: UndefVarError: `decompress_symmetric` not defined in `Main` Stacktrace: [1] top-level scope @ ~/JuliaPkgs/NonconvexSemidefinite.jl/test/runtests.jl:6 [2] macro expansion ``` But if I paste the code into the REPL ``` julia> decompress_symmetric ERROR: UndefVarError: `decompress_symmetric` not defined in `Main` Hint: It looks like two or more modules export different bindings with this name, resulting in ambiguity. Try explicitly importing it from a particular module, or qualifying the name with the module it should come from. Hint: a global variable of this name also exists in NonconvexCore. Hint: a global variable of this name also exists in NonconvexSemidefinite. Hint: a global variable of this name also exists in NonconvexIpopt. ``` which makes me immediately understand what is going on. This just moves the code for this error hint to live beside all the other hints, I see no reason for keeping this special. (cherry picked from commit 1431bec)
Co-authored-by: Jishnu Bhattacharya <jishnub.github@gmail.com> (cherry picked from commit 3e57a8a)
Fixes #57702. We're calling cl-convert- on `using` and `import` statements when we shouldn't, so if there's a nearby local that gets boxed (recursive function definition in this case), and the local shares a name with something in an import statement, we get a box access where we want a raw symbol. Before: ``` julia> let; let; import SHA: R; end; let; R(x...) = R(x); end; end ERROR: TypeError: in import, expected Symbol, got a value of type Expr Stacktrace: [1] top-level scope @ REPL[1]:1 ``` After: ``` julia> let; let; import SHA: R; end; let; R(x...) = R(x); end; end (::var"#R#R##0") (generic function with 1 method) ``` Previously, symbols in `import`/`using` statements would be wrapped with `outerref`, which cl-convert- wouldn't peek into. This protected us from this problem in 1.11. (cherry picked from commit ca17927)
(cherry picked from commit e14fb0c)
This was an oversight. (cherry picked from commit 17f99bc)
Make sure that non-ASCII in the big string macro correctly throws an ArgumentError instead of a string indexing error. (cherry picked from commit 246c93b)
When getting stacktraces on non-X86 platforms, the first frame may not have been set up yet, incorrectly triggering this bad-frame detection logic. This should fix the issue of async unwind failing after only getting 2 frames, if the first frame happens to land in the function header. This is not normally an issue on X86 or non-signals, but also causes no expected issues to be the same logic there too. Fix #52334 After (on arm64-apple-darwin24.3.0): ``` julia> f(1) Warning: detected a stack overflow; program state may be corrupted, so further execution might be unreliable. ERROR: StackOverflowError: Stacktrace: [1] f(x::Int64) @ Main ./REPL[3]:1 [2] g(x::Int64) @ Main ./REPL[4]:1 --- the above 2 lines are repeated 39990 more times --- [79983] f(x::Int64) @ Main ./REPL[3]:1 ``` n.b. This will not fix and is not related to any issues where profiling gets only a single stack frame during profiling of syscalls on Apple AArch64. This fix is specific to the bug where it gets exactly 2 frames. (cherry picked from commit f82917a)
Widening from Float32 to Float64 and then rounding to Float16 will not introduce any error, but going from Float64 -> Float32 -> Float16 will round incorrectly if the intermediate Float32 is halfway between two Float16s. Fixes #57805. Thanks to @vtjnash for suggesting the fix. Co-authored-by: Jameson Nash <jameson@juliacomputing.com> (cherry picked from commit a676b12)
…HS to be global, and prohibit "const x[] = 1" (#57470) The changes to `const` resulted in confusing error messages when it was used inside a function (#57334). On 1.11.3: ``` julia> function f() const x = 1 end ERROR: syntax: unsupported `const` declaration on local variable around REPL[1]:2 Stacktrace: [1] top-level scope @ REPL[1]:1 ``` On nightly: ``` julia> function f() const x = 1 end ERROR: syntax: World age increment not at top level Stacktrace: [1] top-level scope @ REPL[1]:1 ``` In prior versions, we also accepted confused expressions like: ``` x = Ref(1) const x[] = 1 ``` This change adds a new error messages explicitly prohibiting `const` where the left hand side is not declaring variables: ``` ERROR: syntax: `const` left hand side "x[]" contains non-variables around REPL[2]:1 Stacktrace: [1] top-level scope @ REPL[2]:1 ``` Finally, #54773 made `const` stop participating in scope resolution (the left hand side was always taken to be in global scope). Some expressions that were prohibited started being accepted: In 1.11.3: ``` julia> let const x = 1 end ERROR: syntax: unsupported `const` declaration on local variable around REPL[1]:2 Stacktrace: [1] top-level scope @ REPL[1]:1 ``` Nightly: ``` julia> let const x = 1 end 1 ``` This change rejects `const` unless the variable would be in global scope (`global const` would be required in the example), so we don't lose the ability to make `const` in local scope meaningful later. (cherry picked from commit fb01f91)
With the "classic" inference timing disabled, PrecompileTools and SnoopCompile are non-functional on 1.12 & master. #57074 added timing data to the CodeInstances themselves, which should help restore SnoopCompile. However, without the tree structure provided by the old inference timing system, we still need a way to tag MethodInstances that get inferred inside `PrecompileTools.@compile_workload` blocks. This adds a simple mechanism modeled after `@trace_compile` that switches to tagging MethodInstances in `jl_push_newly_inferred`. JuliaLang/PrecompileTools.jl#47 contains (or will contain) the corresponding changes to PrecompileTools.jl. (cherry picked from commit c89b1ff)
Restricts the dispatch for these macros so that only `String` (literal) arguments are accepted: * `int128_str` * `uint128_str` * `big_str` * `ip_str` from the Sockets stdlib * `dateformat_str` from the Dates stdlib Some of these changes make the code in the sysimage less vulnerable to invalidation. (cherry picked from commit f9365a5)
We could make this have a nicer API but for now lets just do what we've done for pkgimages/sysimages and use the envar (cherry picked from commit 17fff87)
Both `--trim=no` and `--trim=safe` are supposed to be safe options by default, so let's not get overzealous and throw out all the IR even though we're not trimming Allows `juliac.jl` to use/test the sysimage creation pipeline unrelated to `--trim` support, as in #57656 (comment) (cherry picked from commit f9e5af1)
Although a package is loaded, we may not have top-level `using` rights. Instead, we need to pull the Module straight out of the loaded modules array in order to do these hacks. (cherry picked from commit 1cead2b)
As recommended in #57589 (comment) (cherry picked from commit eba2a33)
Adds 4 new Float16 fields to CodeInstance to replace Compiler.Timings with continually collected and available measurements. Sample results on a novel method signature: julia> @time code_native(devnull, ÷, dump_module=false, (Int32, UInt16)); 0.006262 seconds (3.62 k allocations: 186.641 KiB, 75.53% compilation time) julia> b = which(÷, (Int32, UInt16)).specializations[6].cache CodeInstance for MethodInstance for div(::Int32, ::UInt16) julia> reinterpret(Float16, b.time_infer_self) Float16(0.0002766) julia> reinterpret(Float16, b.time_infer_total) Float16(0.00049) julia> reinterpret(Float16, b.time_infer_cache_saved) Float16(0.02774) julia> reinterpret(Float16, b.time_compile) Float16(0.003773) Closes #56115 (cherry picked from commit 18b5d8f)
Otherwise inspecting it from julia will be wrong/UB (cherry picked from commit 0df5ad7)
Add an extra indirection to our printing code for `@assert` so that that inferrability is restored after the override. Also partially revert d77c24f which, deleted init code important for JLL's ~This still requires that we bypass this inference-disabling code in JLLWrappers.jl:~ ```julia if isdefined(Base, :Experimental) && isdefined(Base.Experimental, Symbol("@compiler_options")) @eval Base.Experimental.@compiler_options compile=min optimize=0 infer=false end ``` (cherry picked from commit d369c10)
3 interrelated fixes here: - Re-word the confusing error message for a circular dependency error. - Calling `using` inside a package was previously declared to be an error, but never really enforced, so allow it explicitly to do what it logically means (when appearing explicitly, such as from having `Requires.@require` in an `__init__` function). Remove the associated warning in the docs, since this gotcha now simply means what users think it would mean. - A missing lock acquire in create_expr_cache lead to a confusing discrepancy in errors related to the previous two fixes between incremental and serial modes. Fixes #56742 (cherry picked from commit 7f8ca29)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backported PRs:
Base.summarysize
forMemory
withUnion
#57508Rational
fromAbstractIrrational
#55853circshift!(::AbstractVector, ::Integer)
#57539>:
inwhere
#57554isconst(::GlobalRef)
#57563jl_init__threading
andjl_init_with_image__threading
#57561function
declaration implicitlyglobal
#57562Base.Precompilation.ExplicitEnv
: handle type instability better in constructor #57599nextind
,prevind
methods #57608AnnotatedString
: add concrete type asserts toisvalid
,ncodeunits
#57607Base
:power_by_squaring
: don't requireone
#57590printvalue
#57584OncePerX
more aggressively #57660force_enable_inference
to override Module-level inference settings #57653abstract_apply
: declare type of two closure captures #57684Base
:append!
,resize!
: convert length toInt
before passing it on #57585_lift_svec_ref
: improve type stability #57633string(::LazyLibraryPath)
#57721Tuple
instead ofVector
#57734Core.SimpleVector
intypejoin_union_tuple
#57631::SimpleVector
in_collapse_repeated_frames
#57691import
orusing
#57774!
in a strip-metadata case #57778::Real
fallback stack overflow #57790String
#57781const
inside a function, require the LHS to be global, and prohibit "const x[] = 1" #57470--strip-ir
when compiling with--trim=no
#57659Need manual backport:
reinit_ccallable
#56987Non-merged PRs with backport label:
macro
methods for pre-compilation #57833@nospecialize
forstring_index_err
#57604maxthreadid
fromThreads
#57490