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

Dead code elimination in type inference #16011

Merged
merged 1 commit into from
Apr 23, 2016
Merged

Dead code elimination in type inference #16011

merged 1 commit into from
Apr 23, 2016

Conversation

yuyichao
Copy link
Contributor

Fix #15898

The example in that issue now gives

julia> function f(x)
           if true
               sin(x)
           else
               cos(x)
           end
       end
f (generic function with 1 method)

julia> @code_warntype f(1.0)
Variables:
  #self#::#f
  x::Float64

Body:
  begin  # REPL[1], line 2:
      true # REPL[1], line 3:
      return (Main.sin)(x::Float64)::Float64
  end::Float64

julia> @code_llvm f(1.0)

define double @julia_f_53305(double) #0 {
top:
  %1 = call double @julia_sin_53306(double %0) #0
  ret double %1
}

Deleting unreachable code in type_annotate! breaks two assumptions.

  1. The jump target of Expr(:gotoifnot) might be deleted.

    This is fixed up in reindex_labels! and I think this is the right place.

  2. The last expression may not be Expr(:return)

    This is currently only (AFAIK) being relied on in the inlining pass when computing the return value so
    I fixed it there. If there are other places where we have this assumption this can also be fixed in the
    dead code elimination itself by adding a return at the end although I couldn't figure out what's the
    right expression to return in that case...

@JeffBezanson
Copy link
Sponsor Member

Very nice. LGTM.

If all return statements in a function are unreachable, then it doesn't return, and I think we will tolerate this and just mark the final basic block as unreachable.

@yuyichao
Copy link
Contributor Author

Hmm, this can create a mismatch in push and pop of inbounds for the case in #13461

julia> function f(a)
           @inbounds return a[1]
       end
f (generic function with 1 method)

julia> @code_warntype f([1])
Variables:
  #self#::#f
  a::Array{Int64,1}

Body:
  begin  # REPL[1], line 2:
      $(Expr(:inbounds, true))
      return (Base.arrayref)(a::Array{Int64,1},1)::Int64
  end::Int64

It's not hard to check for inbounds expression (Although it can also be in a return like this case...) Do we have other expression that needs to be treated this way? (Or is there a better way to handle this in general?)

@yuyichao yuyichao force-pushed the yyc/elim-dead branch 3 times, most recently from b5b9859 to 84f5001 Compare April 23, 2016 01:37
@yuyichao
Copy link
Contributor Author

Added the check for Expr(:inbounds) and Expr(:boundscheck) (which are the only two types of expressions that pushs and pops a stack in codegen after type inference finishes) and added tests that fails on the previous version of this PR.

The check for the non-deletable expressions can probably be simplified when we have very linear IRs...

@blakejohnson
Copy link
Contributor

Those are some evil tests, but LGTM.

@yuyichao yuyichao added the performance Must go faster label Apr 23, 2016
@yuyichao yuyichao merged commit b12c9fb into master Apr 23, 2016
@yuyichao yuyichao deleted the yyc/elim-dead branch April 23, 2016 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Constant folding in typeinf produce worse code
3 participants