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

effects: add effects analysis for array construction #46015

Merged
merged 1 commit into from
Aug 5, 2022

Conversation

aviatesk
Copy link
Member

This commit implements an infrastructure to analyze effects of
:foreigncall expression, and especially implements effects
analysis for array constructions.

Some improvements:

julia> @noinline construct_array(@nospecialize(T), args...) = Array{T}(undef, args...);

julia> function check_dims(T, dims)
           construct_array(T, dims...)
           return  nothing
       end;
julia> code_typed() do
           check_dims(Int, (1,2,3))
       end
diff --git a/_master b/_pr
index b0ed0eaac1..38e2d3553d 100644
--- a/_master
+++ b/_pr
@@ -1,6 +1,4 @@
 1-element Vector{Any}:
  CodeInfo(
-1 ─      invoke Main.construct_array(Int64::Any, -1::Int64, 2::Vararg{Int64}, 3)::Any
-│   %2 = Main.nothing::Nothing
-└──      return %2
+1 ─     return nothing
 ) => Nothing
julia> code_typed() do
          check_dims(Int, (-1,2,3))
      end
diff --git a/_master b/_pr
index b0ed0eaac1..e5e724db10 100644
--- a/_master
+++ b/_pr
@@ -1,6 +1,5 @@
 1-element Vector{Any}:
  CodeInfo(
-1 ─      invoke Main.construct_array(Int64::Any, -1::Int64, 2::Vararg{Int64}, 3)::Any
-│   %2 = Main.nothing::Nothing
-└──      return %2
-) => Nothing
+1 ─     invoke Main.check_dims(Main.Int::Type, (-1, 2, 3)::Tuple{Int64, Int64, Int64})::Union{}
+└──     unreachable
+) => Union{}

@oscardssmith
Copy link
Member

oscardssmith commented Jul 13, 2022

Can you check whether this improves the time for allocating small arrays Iref
#45160)? On master, I'm seeing

julia> @btime Ref(Int);
  4.740 ns (1 allocation: 16 bytes)

julia> @btime Int[];
  21.435 ns (1 allocation: 64 bytes)

@aviatesk
Copy link
Member Author

It shouldn't impact the time to allocate arrays. This is mostly for dead code elimination.

@Keno
Copy link
Member

Keno commented Jul 13, 2022

I would prefer if these used the existing capability to annotate effects on :foreigncalls rather than hacking this into abstract interpretation.

@aviatesk
Copy link
Member Author

I would prefer if these used the existing capability to annotate effects on :foreigncalls rather than hacking this into abstract interpretation.

I found something like

Array{T,1}(::UndefInitializer, m::Int) where {T} = (@_total_but_inconsistent_meta;
    ccall(:jl_alloc_array_1d, Array{T,1}, (Any, Int), Array{T,1}, m))

is wrong as Array{Int,1}(undef, -1) for example throws, but I still wanted to make it :nothrow in order to make it is_removable_if_unused.
To Implement the analysis in abstract interpretation has another benefit that we can taint it as consistent = TRISTATE_UNKNOWN letting adjust_effects to refine the tainted :consistent-cy to ALWAYS_TRUE when the constructed array is never returned (#45701).

@aviatesk
Copy link
Member Author

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

@nanosoldier
Copy link
Collaborator

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

@aviatesk
Copy link
Member Author

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

@nanosoldier
Copy link
Collaborator

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

@aviatesk
Copy link
Member Author

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

@nanosoldier
Copy link
Collaborator

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

@aviatesk
Copy link
Member Author

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

@nanosoldier
Copy link
Collaborator

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

@aviatesk aviatesk force-pushed the avi/array-effects branch 2 times, most recently from 15ca11c to db391cc Compare August 1, 2022 02:58
@aviatesk
Copy link
Member Author

aviatesk commented Aug 2, 2022

@nanosoldier runtests(["ASE", "CairoMakie", "CountdownNumbers", "DrelTools", "GraphNeuralNetworks", "IncrementalPruning", "KernelEstimator", "PoreMatMod", "ReinforcementLearningExperiments", "RetroCap", "StrBase", "SunAsAStar", "TopoPlots", "TuringGLM", "AbstractLogic", "FlameGraphs", "Folds", "GraphMLDatasets", "HYPRE", "HighDimPDE", "InformationGeometry", "Lux", "ProfileView", "StenoGraphs", "StressTest", "COPT", "Pidfile", "SBML"], vs = ":master")

@nanosoldier
Copy link
Collaborator

Something went wrong when running your job:

Base.IOError("unlink(\"/tmp/jl_c8vjoA\"): no such file or directory (ENOENT)", -2)

cc @maleadt

@aviatesk aviatesk force-pushed the avi/array-effects branch 2 times, most recently from 516ee0a to 23a3935 Compare August 4, 2022 05:09
This commit implements an infrastructure to analyze effects of
`:foreigncall` expression, and especially implements effects
analysis for array constructions.

Some improvements:
```julia
julia> @noinline construct_array(@nospecialize(T), args...) = Array{T}(undef, args...);

julia> function check_dims(T, dims)
           construct_array(T, dims...)
           return  nothing
       end;
```

```julia
julia> code_typed() do
           check_dims(Int, (1,2,3))
       end
```
```diff
diff --git a/_master b/_pr
index b0ed0eaac1..38e2d3553d 100644
--- a/_master
+++ b/_pr
@@ -1,6 +1,4 @@
 1-element Vector{Any}:
  CodeInfo(
-1 ─      invoke Main.construct_array(Int64::Any, -1::Int64, 2::Vararg{Int64}, 3)::Any
-│   %2 = Main.nothing::Nothing
-└──      return %2
+1 ─     return nothing
 ) => Nothing
```

```julia
julia> code_typed() do
          check_dims(Int, (-1,2,3))
      end
```
```diff
diff --git a/_master b/_pr
index b0ed0eaac1..e5e724db10 100644
--- a/_master
+++ b/_pr
@@ -1,6 +1,5 @@
 1-element Vector{Any}:
  CodeInfo(
-1 ─      invoke Main.construct_array(Int64::Any, -1::Int64, 2::Vararg{Int64}, 3)::Any
-│   %2 = Main.nothing::Nothing
-└──      return %2
-) => Nothing
+1 ─     invoke Main.check_dims(Main.Int::Type, (-1, 2, 3)::Tuple{Int64, Int64, Int64})::Union{}
+└──     unreachable
+) => Union{}
```
@aviatesk
Copy link
Member Author

aviatesk commented Aug 5, 2022

Test failures look unrelated.

@aviatesk aviatesk merged commit 01c0778 into master Aug 5, 2022
@aviatesk aviatesk deleted the avi/array-effects branch August 5, 2022 06:31
@DilumAluthge
Copy link
Member

@aviatesk It looks like this PR introduced some new compiler/effects test failures on i686-linux-gnu.

DilumAluthge added a commit that referenced this pull request Aug 6, 2022
ffucci pushed a commit to ffucci/julia that referenced this pull request Aug 11, 2022
This commit implements an infrastructure to analyze effects of
`:foreigncall` expression, and especially implements effects
analysis for array constructions.

Some improvements:
```julia
julia> @noinline construct_array(@nospecialize(T), args...) = Array{T}(undef, args...);

julia> function check_dims(T, dims)
           construct_array(T, dims...)
           return  nothing
       end;
```

```julia
julia> code_typed() do
           check_dims(Int, (1,2,3))
       end
```
```diff
diff --git a/_master b/_pr
index b0ed0eaac1..38e2d3553d 100644
--- a/_master
+++ b/_pr
@@ -1,6 +1,4 @@
 1-element Vector{Any}:
  CodeInfo(
-1 ─      invoke Main.construct_array(Int64::Any, -1::Int64, 2::Vararg{Int64}, 3)::Any
-│   %2 = Main.nothing::Nothing
-└──      return %2
+1 ─     return nothing
 ) => Nothing
```

```julia
julia> code_typed() do
          check_dims(Int, (-1,2,3))
      end
```
```diff
diff --git a/_master b/_pr
index b0ed0eaac1..e5e724db10 100644
--- a/_master
+++ b/_pr
@@ -1,6 +1,5 @@
 1-element Vector{Any}:
  CodeInfo(
-1 ─      invoke Main.construct_array(Int64::Any, -1::Int64, 2::Vararg{Int64}, 3)::Any
-│   %2 = Main.nothing::Nothing
-└──      return %2
-) => Nothing
+1 ─     invoke Main.check_dims(Main.Int::Type, (-1, 2, 3)::Tuple{Int64, Int64, Int64})::Union{}
+└──     unreachable
+) => Union{}
```
ffucci pushed a commit to ffucci/julia that referenced this pull request Aug 11, 2022
pcjentsch pushed a commit to pcjentsch/julia that referenced this pull request Aug 18, 2022
This commit implements an infrastructure to analyze effects of
`:foreigncall` expression, and especially implements effects
analysis for array constructions.

Some improvements:
```julia
julia> @noinline construct_array(@nospecialize(T), args...) = Array{T}(undef, args...);

julia> function check_dims(T, dims)
           construct_array(T, dims...)
           return  nothing
       end;
```

```julia
julia> code_typed() do
           check_dims(Int, (1,2,3))
       end
```
```diff
diff --git a/_master b/_pr
index b0ed0eaac1..38e2d3553d 100644
--- a/_master
+++ b/_pr
@@ -1,6 +1,4 @@
 1-element Vector{Any}:
  CodeInfo(
-1 ─      invoke Main.construct_array(Int64::Any, -1::Int64, 2::Vararg{Int64}, 3)::Any
-│   %2 = Main.nothing::Nothing
-└──      return %2
+1 ─     return nothing
 ) => Nothing
```

```julia
julia> code_typed() do
          check_dims(Int, (-1,2,3))
      end
```
```diff
diff --git a/_master b/_pr
index b0ed0eaac1..e5e724db10 100644
--- a/_master
+++ b/_pr
@@ -1,6 +1,5 @@
 1-element Vector{Any}:
  CodeInfo(
-1 ─      invoke Main.construct_array(Int64::Any, -1::Int64, 2::Vararg{Int64}, 3)::Any
-│   %2 = Main.nothing::Nothing
-└──      return %2
-) => Nothing
+1 ─     invoke Main.check_dims(Main.Int::Type, (-1, 2, 3)::Tuple{Int64, Int64, Int64})::Union{}
+└──     unreachable
+) => Union{}
```
pcjentsch pushed a commit to pcjentsch/julia that referenced this pull request Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants