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

Constant folding in typeinf produce worse code #15898

Closed
yuyichao opened this issue Apr 16, 2016 · 4 comments · Fixed by #16011
Closed

Constant folding in typeinf produce worse code #15898

yuyichao opened this issue Apr 16, 2016 · 4 comments · Fixed by #16011
Labels
compiler:codegen Generation of LLVM IR and native code

Comments

@yuyichao
Copy link
Contributor

yuyichao commented Apr 16, 2016

When type inference detects a branch condition is constant, the not taken branch will not be inferred and therefore all the types are left to be Any. Unfortunately, codegen is not using this info and therefore emit code for uninferred AST which addes unused GC frame to the function and cause the emitted code to be worse.

Example, when const prop doesn't work

julia> function f1(a)
           if 1 == 1
               cos(a)
           else
               sin(a)
           end
       end
f1 (generic function with 1 method)

julia> @code_warntype f1(1.0)
Variables:
  #self#::#f1
  a::Float64

Body:
  begin  # REPL[1], line 2:
      unless (1 === 1)::Bool goto 5 # REPL[1], line 3:
      return (Base.Math.nan_dom_err)((top(ccall))((top(tuple))("cos",Base.Math.libm)::Tuple{ASCIIString,ASCIIString},Base.Math.Float64,(top(svec))(Base.Math.Float64)::SimpleVector,a::Float64,0)::Float64,a::Float64)::Float64
      5:  # REPL[1], line 5:
      return (Base.Math.nan_dom_err)((top(ccall))((top(tuple))("sin",Base.Math.libm)::Tuple{ASCIIString,ASCIIString},Base.Math.Float64,(top(svec))(Base.Math.Float64)::SimpleVector,a::Float64,0)::Float64,a::Float64)::Float64
  end::Float64
define double @julia_f1_31288(double) #0 {
top:
  %1 = call double inttoptr (i64 139969411392000 to double (double)*)(double %0)
  %2 = fcmp uno double %0, 0.000000e+00
  %3 = fcmp ord double %1, 0.000000e+00
  %4 = or i1 %2, %3
  br i1 %4, label %pass, label %fail

fail:                                             ; preds = %top
  %5 = load %jl_value_t*, %jl_value_t** @jl_domain_exception, align 8
  call void @jl_throw(%jl_value_t* %5)
  unreachable

pass:                                             ; preds = %top
  ret double %1
}

When const prop works,

julia> function f2(a)
           if true
               cos(a)
           else
               sin(a)
           end
       end
f2 (generic function with 1 method)

julia> @code_warntype f2(1.0)
Variables:
  #self#::#f2
  a::Float64

Body:
  begin  # REPL[2], line 2:
      unless true goto 5 # REPL[2], line 3:
      return (Base.Math.nan_dom_err)((top(ccall))((top(tuple))("cos",Base.Math.libm)::Tuple{ASCIIString,ASCIIString},Base.Math.Float64,(top(svec))(Base.Math.Float64)::SimpleVector,a::Float64,0)::Float64,a::Float64)::Float64
      5:  # REPL[2], line 5:
      return (Main.sin)(a::Float64)::Any
  end::Float64
define double @julia_f2_31290(double) #0 {
top:
  %1 = alloca [5 x %jl_value_t*], align 8
  %.sub = getelementptr inbounds [5 x %jl_value_t*], [5 x %jl_value_t*]* %1, i64 0, i64 0
  %2 = getelementptr [5 x %jl_value_t*], [5 x %jl_value_t*]* %1, i64 0, i64 3
  %3 = getelementptr [5 x %jl_value_t*], [5 x %jl_value_t*]* %1, i64 0, i64 2
  store %jl_value_t* null, %jl_value_t** %3, align 8
  store %jl_value_t* null, %jl_value_t** %2, align 8
  %4 = getelementptr [5 x %jl_value_t*], [5 x %jl_value_t*]* %1, i64 0, i64 4
  store %jl_value_t* null, %jl_value_t** %4, align 8
  %5 = bitcast [5 x %jl_value_t*]* %1 to i64*
  store i64 6, i64* %5, align 8
  %6 = getelementptr [5 x %jl_value_t*], [5 x %jl_value_t*]* %1, i64 0, i64 1
  %7 = load i64, i64* bitcast (%jl_value_t*** @jl_tls_states to i64*), align 8
  %8 = bitcast %jl_value_t** %6 to i64*
  store i64 %7, i64* %8, align 8
  store %jl_value_t** %.sub, %jl_value_t*** @jl_tls_states, align 8
  %9 = call double inttoptr (i64 139969411392000 to double (double)*)(double %0)
  %10 = fcmp uno double %0, 0.000000e+00
  %11 = fcmp ord double %9, 0.000000e+00
  %12 = or i1 %10, %11
  br i1 %12, label %pass, label %fail

fail:                                             ; preds = %top
  %13 = load %jl_value_t*, %jl_value_t** @jl_domain_exception, align 8
  call void @jl_throw(%jl_value_t* %13)
  unreachable

pass:                                             ; preds = %top
  %14 = load i64, i64* %8, align 8
  store i64 %14, i64* bitcast (%jl_value_t*** @jl_tls_states to i64*), align 8
  ret double %9
}
@vtjnash
Copy link
Member

vtjnash commented Apr 17, 2016

maybe we should run llvm's dead branch elimination pass before gc-root allocation? we don't want to run passes that might rearrange the instructions and make the job of the allocator harder, but that seems safe enough to me, and would help catch these cases fast.

@yuyichao
Copy link
Contributor Author

Can this somehow be handled in type inference? The typed AST also becomes more scary due to the ::Any's in it...

@carnaval
Copy link
Contributor

the correct answer is Bottom for dead code, this definitely looks like something type inference should do

@JeffBezanson JeffBezanson added the compiler:codegen Generation of LLVM IR and native code label Apr 17, 2016
@JeffBezanson
Copy link
Member

In type_annotate! we could mark everything in un-reached statements as Bottom.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants