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

Const bindings not optimized in local scope functions #13395

Closed
mbauman opened this issue Oct 1, 2015 · 2 comments
Closed

Const bindings not optimized in local scope functions #13395

mbauman opened this issue Oct 1, 2015 · 2 comments

Comments

@mbauman
Copy link
Member

mbauman commented Oct 1, 2015

I feel like I've seen this issue reported before (functions hidden behind let declarations have a performance penalty), but I can't find it now. And I'm not sure that it was reduced to a minimal example.

julia> const x = 10
       f() = x*x
       @code_llvm f()

define i64 @julia_f_22102() {
top:
  ret i64 100
}

julia> g = let
           const x = 10
           g() = x*x
       end
       @code_llvm g()

define %jl_value_t* @julia_g_22119(%jl_value_t*, %jl_value_t**, i32) {
top:
  %3 = getelementptr inbounds %jl_value_t* %0, i64 1, i32 0
  %4 = load %jl_value_t** %3, align 8
  %5 = getelementptr inbounds %jl_value_t* %4, i64 1, i32 0
  %6 = load %jl_value_t** %5, align 8
  %7 = getelementptr inbounds %jl_value_t* %6, i64 0, i32 0
  %8 = load %jl_value_t** %7, align 8
  %9 = icmp eq %jl_value_t* %8, null
  br i1 %9, label %err, label %ok2

err:                                              ; preds = %top
  call void @jl_undefined_var_error(%jl_value_t* inttoptr (i64 13193094808 to %jl_value_t*))
  unreachable

ok2:                                              ; preds = %top
  %10 = bitcast %jl_value_t* %8 to i64*
  %11 = load i64* %10, align 16
  %12 = mul i64 %11, %11
  %13 = call %jl_value_t* @jl_box_int64(i64 signext %12)
  ret %jl_value_t* %13
}

It seems like we're missing a relatively simple optimization here — is Julia simply forgetting that x is const in this case? Or is this more complicated than I'm making it out to be?

Use-case: over in Benchmarks.jl, we'd like to support running @benchmark macro within a for loop. This involves defining a local benchmarking function(s). In order to prevent benchmarking dynamic dispatch, we assign any non-constant bindings in the expression to temporary constant bindings. For the most part, this works very well. But we'd prefer to avoid strange performance effects like these.

@yuyichao
Copy link
Contributor

yuyichao commented Oct 1, 2015

I think this is a combination of #11159 (or more general #3440) and #5148.

@mbauman
Copy link
Member Author

mbauman commented Oct 1, 2015

Right, of course, it's definitely at least #5148. Thanks @yuyichao.

julia> let
       const x = 1
       x += 1
       end
2

@mbauman mbauman closed this as completed Oct 1, 2015
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

No branches or pull requests

2 participants