optimizer: support callsite annotations of @inline and @noinline

[dghosef]

EDIT: 2021/06/19

This PR:

• supports callsites annotations of both @inline and @noinline
• supports @inline and @noinline annotation in a function body (typically at the top of its body)

fix #18773

---

Current, @noinline only works for function definitions. This PR is my attempt to modify it to be able to direct the compiler to ignore inlining at a function call as discussed in issue #18773. Unfortunately, my current version produces significantly longer LLVM IR and native code and benchmarks much worse than if the function had been marked @noinline at its definition as shown below.

julia> @inline inlined(x) = x
inlined (generic function with 1 method)

julia> @noinline not_inlined(x) = x
not_inlined (generic function with 1 method)

julia> call_not_inlined(x) = not_inlined(x)
call_not_inlined (generic function with 1 method)

julia> override_inlined(x) = @noinline inlined(x)
override_inlined (generic function with 1 method)

julia> code_llvm(call_not_inlined, (Int,))
;  @ REPL[3]:1 within `call_not_inlined'

define i64 @julia_call_not_inlined_132(i64 signext %0) #0 {
top:
  %1 = call i64 @j_not_inlined_134(i64 signext %0) #0
  ret i64 %1
}

julia> code_llvm(override_inlined, (Int,))
;  @ REPL[4]:1 within `override_inlined'

define i64 @julia_override_inlined_162(i64 signext %0) #0 {
top:
  %1 = alloca {}*, align 8
  %gcframe2 = alloca [3 x {}*], align 16
  %gcframe2.sub = getelementptr inbounds [3 x {}*], [3 x {}*]* %gcframe2, i64 0, i64 0
  %2 = bitcast [3 x {}*]* %gcframe2 to i8*
  call void @llvm.memset.p0i8.i32(i8* nonnull align 16 dereferenceable(24) %2, i8 0, i32 24, i1 false)
  %thread_ptr = call i8* asm "movq %fs:0, $0", "=r"() #6
  %ptls_i8 = getelementptr i8, i8* %thread_ptr, i64 -32768
  %3 = bitcast [3 x {}*]* %gcframe2 to i64*
  store i64 4, i64* %3, align 16
  %4 = getelementptr inbounds [3 x {}*], [3 x {}*]* %gcframe2, i64 0, i64 1
  %5 = bitcast i8* %ptls_i8 to i64*
  %6 = load i64, i64* %5, align 8
  %7 = bitcast {}** %4 to i64*
  store i64 %6, i64* %7, align 8
  %8 = bitcast i8* %ptls_i8 to {}***
  store {}** %gcframe2.sub, {}*** %8, align 8
  %9 = call nonnull {}* @jl_box_int64(i64 signext %0)
  %10 = getelementptr inbounds [3 x {}*], [3 x {}*]* %gcframe2, i64 0, i64 2
  store {}* %9, {}** %10, align 16
  store {}* %9, {}** %1, align 8
  %11 = call nonnull {}* @jl_apply_generic({}* inttoptr (i64 140286855463048 to {}*), {}** nonnull %1, i32 1)
  %12 = bitcast {}* %11 to i64*
  %13 = load i64, i64* %12, align 8
  %14 = load i64, i64* %7, align 8
  store i64 %14, i64* %5, align 8
  ret i64 %13
}

julia> @benchmark override_inlined(x) setup=(x=rand())
BenchmarkTools.Trial: 
  memory estimate:  16 bytes
  allocs estimate:  1
  --------------
  minimum time:     17.013 ns (0.00% GC)
  median time:      20.193 ns (0.00% GC)
  mean time:        21.201 ns (0.63% GC)
  maximum time:     491.021 ns (95.50% GC)
  --------------
  samples:          10000
  evals/sample:     998

julia> @benchmark call_not_inlined(x) setup=(x=rand())
BenchmarkTools.Trial: 
  memory estimate:  0 bytes
  allocs estimate:  0
  --------------
  minimum time:     2.223 ns (0.00% GC)
  median time:      2.408 ns (0.00% GC)
  mean time:        2.439 ns (0.00% GC)
  maximum time:     17.312 ns (0.00% GC)
  --------------
  samples:          10000
  evals/sample:     1000

I suspect my modification to inlining.jlis flawed. Currently, my version just skips the body of the loop in assemble_inline_todo! for function calls that have been marked as @noinline (see here.) and as a result probably skips some other important step in the optimization process, but I can't seem to figure out what. I would really appreciate any pointers in the right direction.

[vchuravy]

Would be great to also add callsite inlining in one go.

[dghosef]

Yes, I would like to take a stab at callsite inlining after I finish the noinline, but callsite inlining is a little bit more complicated(although it would probably be more useful to more people) so I figured I would get noinline working first

[dghosef]

Just fixed problems with previous commit, @noinline now produces expected llvm code as shown below.

julia> @inline inlined(x) = x
inlined (generic function with 1 method)

julia> call_inlined(x) = inlined(x)
call_inlined (generic function with 1 method)

julia> code_llvm(call_inlined, (Int, ))

;  @ REPL[2]:1 within `call_inlined'
define i64 @julia_call_inlined_112(i64 signext %0) #0 {
top:
  ret i64 %0
}

julia> override_inlined(x) = @noinline inlined(x)
override_inlined (generic function with 1 method)

julia> code_llvm(override_inlined, (Int, ))
;  @ REPL[4]:1 within `override_inlined'
define i64 @julia_override_inlined_148(i64 signext %0) #0 {
top:
  %1 = call i64 @j_inlined_150(i64 signext %0) #0
  ret i64 %1
}

[imciner2]

Will the call-site @noinline take precedence over the function marking @inline and prevent it from being inlined?

[dghosef]

Yes the idea is to give the user the ability to opt out of inlining for any function call

[imciner2]

Can you add a test that ensures it overrides it then? (and probably adding tests for the other cases would be best as well - since I don't see any new tests added in here).

[dghosef]

Alright, I've added two tests, one to make sure we can overwrite @inline and one to make sure that something like @noinline f(x) + g(x) only overrides the inline for f. Should I also add a line to NEWS.md for this change?

[imciner2]

and one to make sure that something like @noinline f(x) + g(x) only overrides the inline for f

This is an interesting case - is only inlining the first call a reasonable expectation to make, or would people be more likely to assume that everything in that statement won't be inlined (e.g. the calls to both f and g)? I think my first impressions might be that everything in that statement will be prevented from being inlined, because otherwise we would require constructs like @noinline f(x) + @noinline g(x).

I did a quick test with another simple construct (call chaining), and I think this behavior might also be somewhat surprising to people:

julia> function f(x)
       return x + 5
       end
f (generic function with 1 method)

julia> function g(x)
       return x / 5
       end
g (generic function with 1 method)

julia> function k(x)
       return x * 4
       end
k (generic function with 1 method)

julia> j(x) = f( g( k(x) ) )
j (generic function with 1 method)

julia> code_typed(j, Tuple{Int})
1-element Vector{Any}:
 CodeInfo(
1 ─ %1 = Base.mul_int(x, 4)::Int64
│   %2 = Base.sitofp(Float64, %1)::Float64
│   %3 = Base.div_float(%2, 5.0)::Float64
│   %4 = Base.add_float(%3, 5.0)::Float64
└──      return %4
) => Float64

julia> j(x) = @noinline f( g( k(x) ) )
j (generic function with 1 method)

julia> code_typed(j, Tuple{Int})
1-element Vector{Any}:
 CodeInfo(
1 ─ %1 = invoke Main.k(_2::Int64)::Int64
│   %2 = Base.sitofp(Float64, %1)::Float64
│   %3 = Base.div_float(%2, 5.0)::Float64
│   %4 = Base.add_float(%3, 5.0)::Float64
└──      return %4
) => Float64

julia> j(x) = f( @noinline g( k(x) ) )
j (generic function with 1 method)

julia> code_typed(j, Tuple{Int})
1-element Vector{Any}:
 CodeInfo(
1 ─ %1 = invoke Main.k(_2::Int64)::Int64
│   %2 = Base.sitofp(Float64, %1)::Float64
│   %3 = Base.div_float(%2, 5.0)::Float64
│   %4 = Base.add_float(%3, 5.0)::Float64
└──      return %4
) => Float64

The @noinline I put had an effect only on the call to k and not to g, even when the macro was placed right before g and not just on the outside of the entire function call chain. I wasn't expecting only the inner-most call to be non-inlined, and was probably expecting all the calls to be non-inlined, actually. Either way, I don't think there is a way to get g to not inline itself in this call chain with the current macro.

Do any other languages have callsite inlining prevention that we can compare this behavior against?

Should I also add a line to NEWS.md for this change?

That would probably be a good thing to add.

[dghosef]

Do any other languages have callsite inlining prevention that we can compare this behavior against?

I just found out the Intel Fortran Compiler has a very similar ability( see here) and inlines everything in the statement.

My initial reasoning was that putting @noinline before each call would give the user a little more control over avoiding inlining, but I can definitely see how that could lead to confusion. I reworked the macro a little bit and it now produces the following output:

julia> @inline function f(x)
       x
       end
f (generic function with 1 method)

julia> @inline function g(x)
       x
       end
g (generic function with 1 method)

julia> function h(x)
       @noinline f(x) + g(x)
       end
h (generic function with 1 method)

julia> code_typed(h, Tuple{Int})
1-element Vector{Any}:
 CodeInfo(
1 ─ %1 = invoke Main.f(_2::Int64)::Int64
│   %2 = invoke Main.g(_2::Int64)::Int64
│   %3 = invoke Main.:+(%1::Int64, %2::Int64)::Int64
└──      return %3
) => Int64

julia> function f(x)
       x + 5
       end
f (generic function with 1 method)

julia> function g(x)
       x / 5
       end
g (generic function with 1 method)

julia> function k(x)
       x * 4
       end
k (generic function with 1 method)

julia> j(x) = @noinline f( g( k(x) ) )
j (generic function with 1 method)

julia> code_typed(j, Tuple{Int})
1-element Vector{Any}:
 CodeInfo(
1 ─ %1 = invoke Main.k(_2::Int64)::Int64
│   %2 = invoke Main.g(%1::Int64)::Float64
│   %3 = invoke Main.f(%2::Float64)::Float64
└──      return %3
) => Float64

It does seem like this approach might be the way to go as it would probably avoid confusion, especially since it also mirrors the behavior of the @inbounds macro, and there is precedence for it in the Intel compiler.

[tkf]

What should

@noinline f() do
    body
end

do? Is it equivalent to

(1) noinlined call

g() = body
@noinline f(g)

(2) noinlined do block

@noinline g() = body
f(g)

(3) both

@noinline g() = body
@noinline f(g)

? Also, what about the calls inside the do block body?

I think (1) is the most flexible (so no change in this PR, IIUC) and then provide no-arg version of @inline/@noinline s.t.

@noinline f() do
    @inline
    body
end

is equivalent to

@inline g() = body
@noinline f(g)

Or maybe extend the grammar to allow pre-do macro

@noinline f() @inline do
    body
end

? A do-block only macro syntax would also be required for using it with @opaque.

Ref #35116

[dghosef]

Also, what about the calls inside the do block body?

Not sure if this part is out of the scope of this pull request, but I agree it would be nice to have this, so I just added your suggestion. to have @noinline and @inline affect do blocks(All I did was copy @_inline_meta and @_noinline_meta and added a few tests).

As for the two different syntaxes, I'm actually not sure which one would be better. Would having the macro on the line after the do might be a bit clearer in situations where we want to control both the body and the call like below?

@noinline f() @inline do
    body
end

vs

@noinline f() do
    @inline
    body
end

?

As of right now,

@noinline f() do
    body
end

is equivalent to

g() = body
@noinline f(g)

where body is not affected by the @noinline before f(but body itself can have @noinline calls). I hadn't thought about this part too much, but I agree that it does make sense in my opinion to have @noinline only affect the function call in this scenario so we can have flexibility with controlling the body and the call separately.

[tkf]

Thanks, yes, no-arg @inline sounds like the easiest solution (and also useful for @generated). Does this work with other meta macros like @nospecialize?

[dghosef]

I'll add a few tests soon but I don't think this pr would conflict with other meta macros. One effect of making this change is that I believe this syntax

julia> function f(x)
       @noinline
       body
       end

will now be functionally equivalent to

julia> @noinline function f(x)
       body
       end

Is this desired? My guess is this is probably fine since both notations are pretty clear

[dghosef]

Does this work with other meta macros like @nospecialize?

this, this , and this indicate that adding a meta expression with :noinline or :inline in a function body should be fine since the whole function is scanned for the symbols and as a result their position within the function doesn't matter.

This is an example of argument-less @nospecialize and @noinline working together. Other meta expressions should also work

julia> function a(x)
       @noinline
       @nospecialize
       x
       end
a (generic function with 1 method)

julia> a("string")
"string"

julia> a(3.0)
3.0

julia> a(3)
3

julia> using MethodAnalysis

julia> methodinstances(a)
1-element Vector{Core.MethodInstance}:
 MethodInstance for a(::Any)

julia> b(x) = a(x)
b (generic function with 1 method)

julia> code_typed(b, Tuple{Int})
1-element Vector{Any}:
 CodeInfo(
1 ─ %1 = invoke Main.a(_2::Any)::Int64
└──      return %1
) => Int64

[tkf]

Ah, thanks. I'm kinda puzzled that the meta macros to try to "pack" things in the meta expr

julia> @macroexpand Base.@pure Base.@propagate_inbounds f() = nothing
:(f() = begin
          $(Expr(:meta, :pure, :inline, :propagate_inbounds))
          #= REPL[6]:1 =#
          nothing
      end)

julia> @macroexpand function f()
           Base.@_pure_meta
           Base.@_propagate_inbounds_meta
       end
:(function f()
      #= REPL[7]:1 =#
      #= REPL[7]:2 =#
      $(Expr(:meta, :pure))
      #= REPL[7]:3 =#
      $(Expr(:meta, :inline, :propagate_inbounds))
  end)

Though I agree that the code locations you quoted indicate that they can handle multiple meta exprs.

[dghosef]

I'm not exactly sure why this mechanism would be necessary either. I think it was a style choice to try to contain all the :meta hints together in one expression as it is a little cleaner. I could definitely be wrong but I don't see any functional difference between the two, in which case these changes should be compatible

[dghosef]

Slightly unrelated but do I need to change anything since the buildbot fails the test that working directory is clean because deps/checksums/SuiteSparse.v5.8.1+2.x86_64-unknown-freebsd.tar.gz/ is untracked test?

[aviatesk]

Now #41312 is merged, and this PR needs rebasing and support for a case when a source isn't available at the moment of inlining.
I'd like to work on #41328 rather than rebasing this branch.

[oscardssmith]

In that case, should this be closed?

[dghosef]

Yea I think that would make sense. I'll close it