Skip to content

Conversation

@vchuravy
Copy link
Member

@vchuravy vchuravy commented Jan 28, 2024

@gbaraldi and I were looking through the code today and we were surprised by the usages of get_world_counter.
Inference is primarily executed in a frozen world-age so get_world_counter is often fixed to be jl_typeinf_world,
in most cases it seems we should be using get_world_counter(interp).

Add brief docs to Base.get_world_counter and add Base.tls_wold_age. Furthermore to disambiguate get_world_counter from the inference world rename the abstract interpreter accessor to get_inference_world

@vchuravy vchuravy requested review from aviatesk and vtjnash January 28, 2024 04:20
@vchuravy
Copy link
Member Author

I just noticed that get_world_counter during compilation will return max world.

julia/src/gf.c

Line 32 in 1442303

return ~(size_t)0;
that makes the logic even weirder in places :)

@vtjnash
Copy link
Member

vtjnash commented Jan 28, 2024

Why does the interp have a get_world_counter function? This isn't supposed to be a fixed value, but rather the maximum observable value of jl_world_counter at the start of inference (preventing a data race on that value from leaking into making incorrect optimization results)

@vchuravy
Copy link
Member Author

Why does the interp have a get_world_counter function? This isn't supposed to be a fixed value, but rather the maximum observable value of jl_world_counter at the start of inference

To enable that exactly? get_world_counter is frozen during inference so we enter inference with the last observed value and need to store it somewhere? So that's get_world_counter(interp) is for IIUC

@vchuravy vchuravy force-pushed the vc/world_age_inference branch from 6094c1b to c179c16 Compare January 29, 2024 15:25
@vchuravy
Copy link
Member Author

@vtjnash explained to me yesterday that get_world_counter is like a sequence lock and so the uses in Compiler are to check if someone invalidated the global world.

I confused it with current_task->world which gives you the world you currently execute within.

@vchuravy vchuravy changed the title Make sure get_world_counter uses interp max_world Disambiguate get_world_counter and get_inference_world Jan 29, 2024
@vchuravy vchuravy force-pushed the vc/world_age_inference branch from 5e5c4d2 to c6b8ded Compare February 6, 2024 19:11
@vchuravy vchuravy requested a review from vtjnash February 7, 2024 18:48
@vtjnash vtjnash merged commit e2c8809 into master Feb 7, 2024
@vtjnash vtjnash deleted the vc/world_age_inference branch February 7, 2024 19:36
aviatesk added a commit to aviatesk/JET.jl that referenced this pull request Feb 8, 2024
aviatesk added a commit to aviatesk/JET.jl that referenced this pull request Feb 8, 2024
aviatesk added a commit to aviatesk/JET.jl that referenced this pull request Feb 8, 2024
* adjust to JuliaLang/julia#53088

* update `[benchmark|docs]/Manifest.toml`
aviatesk added a commit to JuliaDebug/Cthulhu.jl that referenced this pull request Feb 8, 2024
aviatesk added a commit to JuliaDebug/Cthulhu.jl that referenced this pull request Feb 8, 2024
aviatesk added a commit to JuliaDebug/Cthulhu.jl that referenced this pull request Feb 8, 2024
aviatesk added a commit to JuliaDebug/Cthulhu.jl that referenced this pull request Feb 8, 2024
aviatesk added a commit to JuliaDebug/Cthulhu.jl that referenced this pull request Feb 8, 2024
aviatesk added a commit to JuliaDebug/Cthulhu.jl that referenced this pull request Feb 8, 2024
aviatesk added a commit to JuliaDebug/Cthulhu.jl that referenced this pull request Feb 8, 2024
vchuravy added a commit to JuliaGPU/GPUCompiler.jl that referenced this pull request Feb 8, 2024
vchuravy added a commit to JuliaGPU/GPUCompiler.jl that referenced this pull request Feb 8, 2024
aviatesk added a commit to JuliaCI/BaseBenchmarks.jl that referenced this pull request Feb 9, 2024
vtjnash pushed a commit to JuliaCI/BaseBenchmarks.jl that referenced this pull request Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compiler:inference Type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants