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

fix invalidations related to ismutable #52170

Merged
merged 4 commits into from
Nov 28, 2023

Conversation

ranocha
Copy link
Member

@ranocha ranocha commented Nov 14, 2023

Related to #52134. It would be nice if the underlying inference issue was fixed but this seems to be a hotfix for now.

I have seen this inference problem occurring in Julia v1.9, v1.10, and current master. For example, on Julia v1.9.3, I get

julia> code_warntype(ismutable, (Function,))
MethodInstance for ismutable(::Function)
  from ismutable(x) @ Base reflection.jl:521
Arguments
  #self#::Core.Const(ismutable)
  x::Function
Body::Any
1nothingnothing%3 = Base.typeof(x)::Type{<:Function}%4 = Base.getproperty(%3, :name)::Any%5 = Base.getproperty(%4, :flags)::Any%6 = (%5 & 0x02)::Any%7 = (%6 == 0x02)::Any
└──      return %7

This causes some invalidations when using OrdinaryDiffEq.

@ranocha ranocha added compiler:inference Type inference compiler:latency Compiler latency labels Nov 14, 2023
@vchuravy vchuravy added backport 1.9 Change should be backported to release-1.9 backport 1.10 Change should be backported to the 1.10 release labels Nov 14, 2023
---------

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
@ranocha
Copy link
Member Author

ranocha commented Nov 21, 2023

Bump

@KristofferC
Copy link
Sponsor Member

KristofferC commented Nov 21, 2023

Tests fail. Did they fail with the earlier version of the fix for this as well?

@ranocha
Copy link
Member Author

ranocha commented Nov 22, 2023

CI was green with the first commit in this PR (before applying the suggestion):

Screenshot 2023-11-22 at 08 50 41

@KristofferC
Copy link
Sponsor Member

Maybe revert to that then?

@ranocha
Copy link
Member Author

ranocha commented Nov 22, 2023

Maybe revert to that then?

Done in ab6db68

@ranocha
Copy link
Member Author

ranocha commented Nov 24, 2023

Test failures like

floatfuncs                                        (8) |        started at 2023-11-22T08:45:13.525
      From worker 7:	ERROR: LoadError: UndefVarError: `@which` not defined in `Main`

(https://buildkite.com/julialang/julia-master/builds/30374#018bf613-7cf7-4ce1-b8b9-3d391bcb6c64/783-1040)

Error During Test at /cache/build/tester-amdci4-12/julialang/julia-master/julia-ab6db68752/share/julia/stdlib/v1.11/SparseArrays/test/ambiguous.jl:47
  Got exception outside of a @test
  UndefVarError: `test_project_toml_formatting` not defined in `Aqua`

(https://buildkite.com/julialang/julia-master/builds/30374#018bf613-7cf7-4ce1-b8b9-3d391bcb6c64/783-1141)

Error in testset SparseArrays/ambiguous:
Test Failed at /cache/build/tester-amdci4-12/julialang/julia-master/tmp/jl_aUFVj1/packages/Aqua/jBmtw/src/deps_compat.jl:59
  Expression: isempty(result)
   Evaluated: isempty(Base.PkgId[Libdl [8f399da3-3557-5675-b5ff-fb832c97cbdb], LinearAlgebra [37e2e46d-f89d-539d-b4ee-838fcccc9c8e], Random [9a3f8284-a2c9-5f02-9a11-845980a1fd5c], Serialization [9e88b42a-f829-5b0c-bbe9-9e923198166b]])

(https://buildkite.com/julialang/julia-master/builds/30374#018bf613-7cf7-4ce1-b8b9-3d391bcb6c64/783-1654)

do not seem to be related to this PR, are they?

@KristofferC KristofferC mentioned this pull request Nov 27, 2023
39 tasks
@ranocha
Copy link
Member Author

ranocha commented Nov 28, 2023

CI passes

@KristofferC KristofferC merged commit df40bab into JuliaLang:master Nov 28, 2023
7 checks passed
KristofferC added a commit that referenced this pull request Dec 2, 2023
Backported PRs:
- [x] #51213 <!-- Wait for other threads to finish compiling before
exiting -->
- [x] #51520 <!-- Make allocopt respect the GC verifier rules with non
usual address spaces -->
- [x] #51598 <!-- Use a simple error when reporting sysimg load
failures. -->
- [x] #51757 <!-- fix parallel peakflop usage -->
- [x] #51781 <!-- Don't make pkgimages global editable -->
- [x] #51848 <!-- allow finalizers to take any locks and yield during
exit -->
- [x] #51847 <!-- add missing wait during Timer and AsyncCondition close
-->
- [x] #50824 <!-- Add some aliasing warnings to docstrings for mutating
functions in Base -->
- [x] #51885 <!-- remove chmodding the pkgimages -->
- [x] #50207 <!-- [devdocs] Improve documentation about building
external forks of LLVM -->
- [x] #51967 <!-- further fix to the new promoting method for
AbstractDateTime subtraction -->
- [x] #51980 <!-- macroexpand: handle const/atomic struct fields
correctly -->
- [x] #51995 <!-- [Artifacts] Pass artifacts dictionary to
`ensure_artifact_installed` dispatch -->
- [x] #52098 <!-- Fix errors in `sort` docstring -->
- [x] #52136 <!-- Bump JuliaSyntax to 0.4.7 -->
- [x] #52140 <!-- Make c func `abspath` consistent on Windows. Fix
tracking path conversion. -->
- [x] #52009 <!-- fix completion that resulted in startpos of 0 for `\\
-->
- [x] #52192 <!-- cap the number of GC threads to number of cpu cores
-->
- [x] #52206 <!-- Make have_fma consistent between interpreter and
compiled -->
- [x] #52027 <!-- fix Unicode.julia_chartransform for Julia 1.10 -->
- [x] #52217 <!-- More helpful error message for empty `cpu_target` in
`Base.julia_cmd` -->
- [x] #51371 <!-- Memoize `cwstring` when used for env lookup /
modification on Windows -->
- [x] #52214 <!-- Turn Method Overwritten Error into a PrecompileError
-- turning off caching -->
- [x] #51895 <!-- Devdocs on fixing precompile hangs, take 2 -->
- [x] #51596 <!-- Reland "Don't mark nonlocal symbols as hidden"" -->
- [x] #51834 <!-- [REPLCompletions] allow symbol completions within
incomplete macrocall expression -->
- [x] #52010 <!-- Revert "Support sorting iterators (#46104)" -->
- [x] #51430 <!-- add support for async backtraces of Tasks on any
thread -->
- [x] #51471 <!-- Fix segfault if root task is NULL -->
- [x] #52194 <!-- Fix multiversioning issues caused by the parallel llvm
work -->
- [x] #51035 <!-- refactor GC scanning code to reflect jl_binding_t are
now first class -->
- [x] #52030 <!-- Bump Statistics -->
- [x] #52189 <!-- codegen: ensure i1 bool is widened to i8 before
storing -->
- [x] #52228 <!-- Widen diagonal var during `Type` unwrapping in
`instanceof_tfunc` -->
- [x] #52182 <!-- jitlayers: replace sharedbytes intern pool with one
that respects alignment -->

Contains multiple commits, manual intervention needed:
- [ ] #51092 <!-- inference: fix bad effects for recursion -->

Non-merged PRs with backport label:
- [ ] #52196 <!-- Fix creating custom log level macros -->
- [ ] #52170 <!-- fix invalidations related to `ismutable` -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
mkitti pushed a commit to mkitti/julia that referenced this pull request Dec 9, 2023
Related to JuliaLang#52134. It would be nice if the underlying inference issue
was fixed but this seems to be a hotfix for now.

I have seen this inference problem occurring in Julia v1.9, v1.10, and
current `master`. For example, on Julia v1.9.3, I get

```julia

julia> code_warntype(ismutable, (Function,))
MethodInstance for ismutable(::Function)
  from ismutable(x) @ Base reflection.jl:521
Arguments
  #self#::Core.Const(ismutable)
  x::Function
Body::Any
1 ─      nothing
│        nothing
│   %3 = Base.typeof(x)::Type{<:Function}
│   %4 = Base.getproperty(%3, :name)::Any
│   %5 = Base.getproperty(%4, :flags)::Any
│   %6 = (%5 & 0x02)::Any
│   %7 = (%6 == 0x02)::Any
└──      return %7
```

This causes some invalidations when `using OrdinaryDiffEq`.
KristofferC pushed a commit that referenced this pull request Dec 12, 2023
Related to #52134. It would be nice if the underlying inference issue
was fixed but this seems to be a hotfix for now.

I have seen this inference problem occurring in Julia v1.9, v1.10, and
current `master`. For example, on Julia v1.9.3, I get

```julia

julia> code_warntype(ismutable, (Function,))
MethodInstance for ismutable(::Function)
  from ismutable(x) @ Base reflection.jl:521
Arguments
  #self#::Core.Const(ismutable)
  x::Function
Body::Any
1 ─      nothing
│        nothing
│   %3 = Base.typeof(x)::Type{<:Function}
│   %4 = Base.getproperty(%3, :name)::Any
│   %5 = Base.getproperty(%4, :flags)::Any
│   %6 = (%5 & 0x02)::Any
│   %7 = (%6 == 0x02)::Any
└──      return %7
```

This causes some invalidations when `using OrdinaryDiffEq`.

(cherry picked from commit df40bab)
@KristofferC KristofferC mentioned this pull request Dec 12, 2023
17 tasks
KristofferC added a commit that referenced this pull request Dec 17, 2023
Backported PRs:
- [x] #51234 <!-- Fix getfield codegen for tuple inputs and unknown
symbol fields. -->
- [x] #52170 <!-- fix invalidations related to `ismutable` -->
- [x] #52342 <!-- Add single-term multiplication for `AbstractQ` on
v1.10 and above -->
- [x] #52333 <!-- bugfix for dot of Hermitian{noncommutative} -->
- [x] #52407 <!-- channels: fix memory ordering violation in iterate -->
- [x] #52405 <!-- Bump LLVM to 15.0.7+10 to fix GC issue -->
- [x] #52441 <!-- Remove `Pkg` dependency from `SuiteSparse_jll` -->
- [x] #52367 <!-- docs: add notes about scratchspaces in depot -->
- [x] #52456 <!-- Make `jl_write_coverage_data` dllexported again -->
- [x] #52294 <!-- GC scheduler refinements -->
- [x] #52359 <!-- make custom log macros work -->
- [x] #52548
@KristofferC KristofferC removed the backport 1.10 Change should be backported to the 1.10 release label Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.9 Change should be backported to release-1.9 compiler:inference Type inference compiler:latency Compiler latency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants