Skip to content

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Aug 1, 2024

In extreme cases, the compiler could mark this function for concrete-eval, even though that is illegal unless the compiler has first deleted this instruction. Otherwise the attempt to concrete-eval will re-run the function repeatedly until it hits a StackOverflow.

Workaround to fix #55147

@aviatesk You might know how to solve this even better, using post-optimization effect refinements? Since we should actually only apply the refinement of terminates=false => terminates=true (and thus allowing concrete eval) if the optimization occurs, and not just in inference thinks the optimization would be legal.

@vtjnash vtjnash added compiler:inference Type inference backport 1.11 Change should be backported to release-1.11 labels Aug 1, 2024
@vtjnash vtjnash requested a review from Keno August 1, 2024 21:56
@KristofferC KristofferC mentioned this pull request Aug 2, 2024
68 tasks
@aviatesk
Copy link
Member

aviatesk commented Aug 2, 2024

Thanks for your help, @vtjnash .

You might know how to solve this even better, using post-optimization effect refinements?

What do you think about a634dd2? I think it might be a good idea to set up a new effect bit for this specific purpose and make post-opt analysis refine it.

@vtjnash
Copy link
Member Author

vtjnash commented Aug 2, 2024

That sounds good to me

@vtjnash
Copy link
Member Author

vtjnash commented Aug 7, 2024

It looks like this CI failure happened consistently. Any idea? I assume just a bad test?

Error in testset REPL:
2024-08-02 07:36:40 EDT	Test Failed at /cache/build/tester-amdci4-15/julialang/julia-master/julia-a634dd2d89/share/julia/stdlib/v1.12/REPL/test/replcompletions.jl:2163
2024-08-02 07:36:40 EDT	  Expression: t isa Core.Const
2024-08-02 07:36:40 EDT	   Evaluated: Core.PartialStruct(Cmd, Any[Vector{String}, Core.Const(false), Core.Const(0x00000000), Core.Const(nothing), Core.Const(""), Core.Const(nothing)]) isa Core.Const
2024-08-02 07:36:40 EDT	
2024-08-02 07:36:40 EDT	Error in testset REPL:
2024-08-02 07:36:40 EDT	Error During Test at /cache/build/tester-amdci4-15/julialang/julia-master/julia-a634dd2d89/share/julia/stdlib/v1.12/REPL/test/replcompletions.jl:2164
2024-08-02 07:36:40 EDT	  Test threw exception
2024-08-02 07:36:40 EDT	  Expression: t.val == `a b`
2024-08-02 07:36:40 EDT	  FieldError: type PartialStruct has no field val
2024-08-02 07:36:40 EDT	  Stacktrace:
2024-08-02 07:36:40 EDT	   [1] getproperty(x::Core.PartialStruct, f::Symbol)
2024-08-02 07:36:40 EDT	     @ Base ./Base.jl:49
2024-08-02 07:36:40 EDT	   [2] top-level scope
2024-08-02 07:36:40 EDT	     @ /cache/build/tester-amdci4-15/julialang/julia-master/julia-a634dd2d89/share/julia/stdlib/v1.12/REPL/test/replcompletions.jl:2164
2024-08-02 07:36:40 EDT	   [3] macro expansion
2024-08-02 07:36:40 EDT	     @ /cache/build/tester-amdci4-15/julialang/julia-master/julia-a634dd2d89/share/julia/stdlib/v1.12/Test/src/Test.jl:676 [inlined]

But seems simple, and thus perhaps supposed to work?

let t = REPLCompletions.repl_eval_ex(:(`a b`), @__MODULE__; limit_aggressive_inference=true)
    @test t isa Core.Const
    @test t.val == `a b`
end

@aviatesk
Copy link
Member

aviatesk commented Aug 9, 2024

I updated the effects annotation for Base.cmd_gen, and it should be fixed now.

@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Aug 9, 2024
Copy link
Member

@aviatesk aviatesk left a comment

Choose a reason for hiding this comment

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

Added docstrings too, should be ready to go.

vtjnash and others added 3 commits August 10, 2024 03:29
In extreme cases, the compiler could mark this function for
concrete-eval, even though that is illegal unless the compiler has first
deleted this instruction. Otherwise the attempt to concrete-eval will
re-run the function repeatedly until it hits a StackOverflow.

Workaround to fix #55147
@aviatesk aviatesk merged commit 7e809b0 into master Aug 10, 2024
7 checks passed
@aviatesk aviatesk deleted the jn/55147 branch August 10, 2024 04:31
aviatesk added a commit that referenced this pull request Aug 10, 2024
In extreme cases, the compiler could mark this function for
concrete-eval, even though that is illegal unless the compiler has first
deleted this instruction. Otherwise the attempt to concrete-eval will
re-run the function repeatedly until it hits a StackOverflow.

Workaround to fix #55147

@aviatesk You might know how to solve this even better, using
post-optimization effect refinements? Since we should actually only
apply the refinement of terminates=false => terminates=true (and thus
allowing concrete eval) if the optimization occurs, and not just in
inference thinks the optimization would be legal.

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
@aviatesk aviatesk removed backport 1.11 Change should be backported to release-1.11 merge me PR is reviewed. Merge when all tests are passing labels Aug 10, 2024
lazarusA pushed a commit to lazarusA/julia that referenced this pull request Aug 17, 2024
…#55338)

In extreme cases, the compiler could mark this function for
concrete-eval, even though that is illegal unless the compiler has first
deleted this instruction. Otherwise the attempt to concrete-eval will
re-run the function repeatedly until it hits a StackOverflow.

Workaround to fix JuliaLang#55147

@aviatesk You might know how to solve this even better, using
post-optimization effect refinements? Since we should actually only
apply the refinement of terminates=false => terminates=true (and thus
allowing concrete eval) if the optimization occurs, and not just in
inference thinks the optimization would be legal.

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
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.

New precompilation crashes on Julia 1.11-rc1
2 participants