effects: taint `:nothrow` effect on unknown `:static_parameter` #46791

aviatesk · 2022-09-16T06:50:30Z

With this commit, we taint :nothrow effect property correctly on access to unknown :static_parameter, e.g.:

unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing)
@test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},))))
@test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,))))

This commit implements a very conservative analysis, and thus there is a room for improvement still, e.g.:

unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing)
@test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,)))

fix #46771

aviatesk · 2022-09-18T05:16:53Z

@Keno is it okay for now to merge this as is, and try to improve the accuracy of this nothrow analysis in a follow up PR (it seems a bit tricky than I expected though)?

Keno · 2022-09-18T07:22:35Z

I would prefer to try to get at least some precision in at the same time, because I suspect we're relying on this bug for performance in a number of applications

staticfloat · 2023-02-07T00:01:47Z

@aviatesk I rebased this branch, so that we can push forward Keno's PR more easily.

vchuravy · 2023-02-07T12:59:53Z

Will this need to be backported to 1.9 since it's a bugfix to the effect system?

aviatesk · 2023-02-07T14:28:44Z

Will this need to be backported to 1.9 since it's a bugfix to the effect system?

Yes, I think we should do it.

aviatesk · 2023-02-07T14:54:48Z

@nanosoldier runbenchmarks("inference", vs=":master")

This is an alternative to #46791. Will be filed as a PR to check the performance difference.

nanosoldier · 2023-02-07T15:49:57Z

Your benchmark job has completed - no performance regressions were detected. A full report can be found here.

oscardssmith · 2023-02-07T16:50:57Z

given that there aren't any regressions here, good to merge?

aviatesk · 2023-02-07T17:05:48Z

There is one test failure on this PR, so don't merge now.

@test

…ervatively) With this commit, we taint `:nothrow` effect property correctly on access to unknown `:static_parameter`, e.g.: ```julia unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing) @test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},)))) @test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,)))) ``` This commit implements a very conservative analysis, and thus there is a room for improvement still, e.g.: ```julia unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing) @test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,))) ```

aviatesk · 2023-02-08T05:55:10Z

@nanosoldier runbenchmarks("inference", vs=":master")

These are only technically unbounded because of the existence of ill-formed types. However, this function is supposed to be an API sanity check and ordinary users should never have ill-formed types, so for the purpose we want here, allow unboundedness in Type{T}.

nanosoldier · 2023-02-08T06:50:24Z

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@test

* effects: taint `:nothrow` effect on unknown `:static_parameter` (conservatively) With this commit, we taint `:nothrow` effect property correctly on access to unknown `:static_parameter`, e.g.: ```julia unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing) @test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},)))) @test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,)))) ``` This commit implements a very conservative analysis, and thus there is a room for improvement still, e.g.: ```julia unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing) @test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,))) ``` * inference: improve `:nothrow` modeling for `:static_parameter` (#46820) * Fix test with free type params * Test: Ignore ::Type{T} in detect_unbounded These are only technically unbounded because of the existence of ill-formed types. However, this function is supposed to be an API sanity check and ordinary users should never have ill-formed types, so for the purpose we want here, allow unboundedness in Type{T}. --------- Co-authored-by: Keno Fischer <[email protected]> Co-authored-by: Elliot Saba <[email protected]> (cherry picked from commit b5d17ea)

@test

* effects: taint `:nothrow` effect on unknown `:static_parameter` (conservatively) With this commit, we taint `:nothrow` effect property correctly on access to unknown `:static_parameter`, e.g.: ```julia unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing) @test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},)))) @test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,)))) ``` This commit implements a very conservative analysis, and thus there is a room for improvement still, e.g.: ```julia unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing) @test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,))) ``` * inference: improve `:nothrow` modeling for `:static_parameter` (#46820) * Fix test with free type params * Test: Ignore ::Type{T} in detect_unbounded These are only technically unbounded because of the existence of ill-formed types. However, this function is supposed to be an API sanity check and ordinary users should never have ill-formed types, so for the purpose we want here, allow unboundedness in Type{T}. --------- Co-authored-by: Keno Fischer <[email protected]> Co-authored-by: Elliot Saba <[email protected]> (cherry picked from commit b5d17ea)

@test

* effects: taint `:nothrow` effect on unknown `:static_parameter` (conservatively) With this commit, we taint `:nothrow` effect property correctly on access to unknown `:static_parameter`, e.g.: ```julia unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing) @test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},)))) @test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,)))) ``` This commit implements a very conservative analysis, and thus there is a room for improvement still, e.g.: ```julia unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing) @test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,))) ``` * inference: improve `:nothrow` modeling for `:static_parameter` (#46820) * Fix test with free type params * Test: Ignore ::Type{T} in detect_unbounded These are only technically unbounded because of the existence of ill-formed types. However, this function is supposed to be an API sanity check and ordinary users should never have ill-formed types, so for the purpose we want here, allow unboundedness in Type{T}. --------- Co-authored-by: Keno Fischer <[email protected]> Co-authored-by: Elliot Saba <[email protected]> (cherry picked from commit b5d17ea)

@test

* effects: taint `:nothrow` effect on unknown `:static_parameter` (conservatively) With this commit, we taint `:nothrow` effect property correctly on access to unknown `:static_parameter`, e.g.: ```julia unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing) @test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},)))) @test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,)))) ``` This commit implements a very conservative analysis, and thus there is a room for improvement still, e.g.: ```julia unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing) @test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,))) ``` * inference: improve `:nothrow` modeling for `:static_parameter` (#46820) * Fix test with free type params * Test: Ignore ::Type{T} in detect_unbounded These are only technically unbounded because of the existence of ill-formed types. However, this function is supposed to be an API sanity check and ordinary users should never have ill-formed types, so for the purpose we want here, allow unboundedness in Type{T}. --------- Co-authored-by: Keno Fischer <[email protected]> Co-authored-by: Elliot Saba <[email protected]> (cherry picked from commit b5d17ea)

@test

* effects: taint `:nothrow` effect on unknown `:static_parameter` (conservatively) With this commit, we taint `:nothrow` effect property correctly on access to unknown `:static_parameter`, e.g.: ```julia unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing) @test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},)))) @test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,)))) ``` This commit implements a very conservative analysis, and thus there is a room for improvement still, e.g.: ```julia unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing) @test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,))) ``` * inference: improve `:nothrow` modeling for `:static_parameter` (#46820) * Fix test with free type params * Test: Ignore ::Type{T} in detect_unbounded These are only technically unbounded because of the existence of ill-formed types. However, this function is supposed to be an API sanity check and ordinary users should never have ill-formed types, so for the purpose we want here, allow unboundedness in Type{T}. --------- Co-authored-by: Keno Fischer <[email protected]> Co-authored-by: Elliot Saba <[email protected]> (cherry picked from commit b5d17ea)

@test

* effects: taint `:nothrow` effect on unknown `:static_parameter` (conservatively) With this commit, we taint `:nothrow` effect property correctly on access to unknown `:static_parameter`, e.g.: ```julia unknown_sparam_throw(::Union{Nothing, Type{T}}) where T = (T; nothing) @test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Type{Int},)))) @test !Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_throw, ((Nothing,)))) ``` This commit implements a very conservative analysis, and thus there is a room for improvement still, e.g.: ```julia unknown_sparam_nothrow(x::Ref{T}) where {T} = (T; nothing) @test_broken Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow, (Ref,))) ``` * inference: improve `:nothrow` modeling for `:static_parameter` (#46820) * Fix test with free type params * Test: Ignore ::Type{T} in detect_unbounded These are only technically unbounded because of the existence of ill-formed types. However, this function is supposed to be an API sanity check and ordinary users should never have ill-formed types, so for the purpose we want here, allow unboundedness in Type{T}. --------- Co-authored-by: Keno Fischer <[email protected]> Co-authored-by: Elliot Saba <[email protected]> (cherry picked from commit b5d17ea)

aviatesk added the compiler:effects effect analysis label Sep 16, 2022

aviatesk force-pushed the avi/46771 branch 4 times, most recently from 7cc9fbb to 14243e3 Compare September 18, 2022 04:46

Keno mentioned this pull request Sep 18, 2022

WIP: Improve nothrow modeling for :static_parameter #46820

Merged

aviatesk force-pushed the avi/46771 branch from 14243e3 to f777202 Compare September 19, 2022 21:08

aviatesk force-pushed the avi/46771 branch from f777202 to 423b939 Compare December 8, 2022 10:24

aviatesk changed the base branch from master to avi/sptypes December 8, 2022 10:24

aviatesk added the DO NOT MERGE Do not merge this PR! label Dec 8, 2022

Base automatically changed from avi/sptypes to master December 9, 2022 01:49

aviatesk force-pushed the avi/46771 branch from 423b939 to dd6aab0 Compare December 9, 2022 01:49

staticfloat force-pushed the avi/46771 branch from dd6aab0 to 860f7c7 Compare February 6, 2023 23:59

aviatesk force-pushed the avi/46771 branch from 860f7c7 to 367b76f Compare February 7, 2023 08:47

aviatesk changed the title ~~effects: taint :nothrow effect on unknown :static_parameter (conservatively)~~ effects: taint :nothrow effect on unknown :static_parameter Feb 7, 2023

aviatesk force-pushed the avi/46771 branch from 0da756b to 790e667 Compare February 7, 2023 14:29

oscardssmith added bugfix This change fixes an existing bug and removed DO NOT MERGE Do not merge this PR! labels Feb 7, 2023

KristofferC added the backport 1.9 Change should be backported to release-1.9 label Feb 7, 2023

aviatesk added a commit that referenced this pull request Feb 7, 2023

keep undefinedness of static parameters as a separate field

99a272a

This is an alternative to #46791. Will be filed as a PR to check the performance difference.

aviatesk mentioned this pull request Feb 7, 2023

keep undefinedness of static parameters as a separate field #48576

Closed

staticfloat force-pushed the avi/46771 branch 3 times, most recently from 14dfe1b to 248fb3c Compare February 8, 2023 00:36

aviatesk and others added 3 commits February 8, 2023 14:53

inference: improve :nothrow modeling for :static_parameter (#46820)

db23898

Fix test with free type params

401567a

aviatesk force-pushed the avi/46771 branch from e1c2d36 to 72dc027 Compare February 8, 2023 05:54

aviatesk force-pushed the avi/46771 branch from 72dc027 to f326fad Compare February 8, 2023 06:18

Keno merged commit b5d17ea into master Feb 8, 2023

Keno deleted the avi/46771 branch February 8, 2023 09:40

KristofferC mentioned this pull request Feb 20, 2023

Backports for 1.9.0-rc1 #48733

Merged

50 tasks

KristofferC removed the backport 1.9 Change should be backported to release-1.9 label Mar 6, 2023

KristofferC mentioned this pull request Mar 16, 2023

call signature with free type variable can corrupt the dispatch cache #49027

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

effects: taint `:nothrow` effect on unknown `:static_parameter` #46791

effects: taint `:nothrow` effect on unknown `:static_parameter` #46791

Uh oh!

aviatesk commented Sep 16, 2022

Uh oh!

aviatesk commented Sep 18, 2022

Uh oh!

Keno commented Sep 18, 2022

Uh oh!

staticfloat commented Feb 7, 2023

Uh oh!

vchuravy commented Feb 7, 2023

Uh oh!

aviatesk commented Feb 7, 2023

Uh oh!

aviatesk commented Feb 7, 2023

Uh oh!

nanosoldier commented Feb 7, 2023

Uh oh!

oscardssmith commented Feb 7, 2023

Uh oh!

aviatesk commented Feb 7, 2023

Uh oh!

aviatesk commented Feb 8, 2023

Uh oh!

nanosoldier commented Feb 8, 2023

Uh oh!

Uh oh!

Uh oh!

effects: taint :nothrow effect on unknown :static_parameter #46791

effects: taint :nothrow effect on unknown :static_parameter #46791

Uh oh!

Conversation

aviatesk commented Sep 16, 2022

Uh oh!

aviatesk commented Sep 18, 2022

Uh oh!

Keno commented Sep 18, 2022

Uh oh!

staticfloat commented Feb 7, 2023

Uh oh!

vchuravy commented Feb 7, 2023

Uh oh!

aviatesk commented Feb 7, 2023

Uh oh!

aviatesk commented Feb 7, 2023

Uh oh!

nanosoldier commented Feb 7, 2023

Uh oh!

oscardssmith commented Feb 7, 2023

Uh oh!

aviatesk commented Feb 7, 2023

Uh oh!

aviatesk commented Feb 8, 2023

Uh oh!

nanosoldier commented Feb 8, 2023

Uh oh!

Uh oh!

effects: taint `:nothrow` effect on unknown `:static_parameter` #46791

effects: taint `:nothrow` effect on unknown `:static_parameter` #46791