jlcall generated for function with only jl_value_t* arguments

[yuyichao]

See below. Somehow if the signature of a function is (::ANY) and if the return type is jl_value_t*, a jlcall is generated even though a c call is generated both when the return type is different and when there's additional parameters to be specialized on.

I don't really know how it is decided whether a jl signature or a c signature will be generated but it seems that this is the wrong decision in this case.

This issue affect #11284. Although I guess I can let the helper function specialize on the first argument to work around this without causing more code to be generated (since the first argument should always be a DataType anyway....)

Test code

@noinline g(::ANY) = 1
@noinline f(::ANY) = Int
@noinline f(::ANY, b) = Int

@code_llvm g(1)
@code_llvm f(1)
@code_llvm f(1, 2)

k1() = g(1)
k2() = f(1)
k3() = f(1, 2)

@code_llvm k1()
@code_llvm k2()
@code_llvm k3()

function time_func(f::Function, args...)
    println(f)
    f(args...)
    gc()
    @time for i in 1:100000000
        f(args...)
    end
    gc()
end

time_func(k1)
time_func(k2)
time_func(k3)

Output

define i64 @julia_g_44382(%jl_value_t*) {
top:
  ret i64 1
}

define %jl_value_t* @julia_f_44385(%jl_value_t*, %jl_value_t**, i32) {
top:
  %3 = load %jl_value_t** inttoptr (i64 139983779531464 to %jl_value_t**), align 8
  ret %jl_value_t* %3
}

define %jl_value_t* @julia_f_44387(%jl_value_t*, i64) {
top:
  %2 = load %jl_value_t** inttoptr (i64 139983779531464 to %jl_value_t**), align 8
  ret %jl_value_t* %2
}

define i64 @julia_k1_44391() {
top:
  %0 = call i64 @julia_g_44382(%jl_value_t* inttoptr (i64 139983779569792 to %jl_value_t*))
  ret i64 %0
}

define %jl_value_t* @julia_k2_44392() {
top:
  %0 = alloca [3 x %jl_value_t*], align 8
  %.sub = getelementptr inbounds [3 x %jl_value_t*]* %0, i64 0, i64 0
  %1 = getelementptr [3 x %jl_value_t*]* %0, i64 0, i64 2
  %2 = bitcast [3 x %jl_value_t*]* %0 to i64*
  store i64 2, i64* %2, align 8
  %3 = getelementptr [3 x %jl_value_t*]* %0, i64 0, i64 1
  %4 = bitcast %jl_value_t** %3 to %jl_value_t***
  %5 = load %jl_value_t*** @jl_pgcstack, align 8
  store %jl_value_t** %5, %jl_value_t*** %4, align 8
  store %jl_value_t** %.sub, %jl_value_t*** @jl_pgcstack, align 8
  store %jl_value_t* inttoptr (i64 139983779569792 to %jl_value_t*), %jl_value_t** %1, align 8
  %6 = call %jl_value_t* @julia_f_44385(%jl_value_t* inttoptr (i64 139983812208976 to %jl_value_t*), %jl_value_t** %1, i32 1)
  %7 = load %jl_value_t*** %4, align 8
  store %jl_value_t** %7, %jl_value_t*** @jl_pgcstack, align 8
  ret %jl_value_t* %6
}

define %jl_value_t* @julia_k3_44393() {
top:
  %0 = call %jl_value_t* @julia_f_44387(%jl_value_t* inttoptr (i64 139983779569792 to %jl_value_t*), i64 2)
  ret %jl_value_t* %0
}
k1
   1.267 seconds     
k2
   1.422 seconds     
k3
   1.280 seconds     

[yuyichao]

It seems that this is not only limited to (::ANY) signature, but any functions that take a single jl_value_t* and returns a jl_value_t*.

[yuyichao]

@vtjnash exactly (you beat me by the time I went to get some water.....) ....

[vtjnash]

yes, i happened to know the location and behavior of the bit of code responsible for the specsig decision:

julia/src/codegen.cpp

Lines 3880 to 3894 in 1b2f4fa

	bool specsig = false;
	if (!va && !hasCapt && lam->specTypes != NULL && lam->inferred) {
	// no captured vars and not vararg
	// consider specialized signature
	for(size_t i=0; i < jl_nparams(lam->specTypes); i++) {
	if (isbits_spec(jl_tparam(lam->specTypes, i))) { // assumes !va
	specsig = true;
	break;
	}
	}
	if (jl_nparams(lam->specTypes) == 0)
	specsig = true;
	if (isbits_spec(jlrettype))
	specsig = true;
	}

[yuyichao]

@vtjnash is there any reason to only specialize the signature in these conditions? Is there trade off with specializing the signatures?

IIRC, there's a limitation on the number of arguments (coming from llvm?) but this is not testing that either...

[yuyichao]

Remove those checks and it seems to work well locally.
Submitted as PR to see what other people and the CI think.

[vtjnash]

unintentionally fixed by jb/functions

[JeffBezanson]

Well, we still have:

julia> type Foo
        x
       end

julia> (f::Foo)() = 1+f.x

julia> @code_llvm Foo(2)()

define %jl_value_t* @julia_Foo_53933(%jl_value_t*, %jl_value_t**, i32) #0 {
top:
...

[yuyichao]

... so it was "fixed" because there's an implicit singleton argument for normal functions now?....