Fix type instability in push!

[mlubin]

Before:

julia> @code_llvm push!([1.0],2)

define %jl_value_t* @"julia_push!_22727"(%jl_value_t*, i64) {
top:
  %2 = alloca [3 x %jl_value_t*], align 8
  %.sub = getelementptr inbounds [3 x %jl_value_t*]* %2, i64 0, i64 0
  %3 = getelementptr [3 x %jl_value_t*]* %2, i64 0, i64 2
  store %jl_value_t* inttoptr (i64 2 to %jl_value_t*), %jl_value_t** %.sub, align 8
  %4 = getelementptr [3 x %jl_value_t*]* %2, i64 0, i64 1
  %5 = load %jl_value_t*** @jl_pgcstack, align 8
  %.c = bitcast %jl_value_t** %5 to %jl_value_t*
  store %jl_value_t* %.c, %jl_value_t** %4, align 8
  store %jl_value_t** %.sub, %jl_value_t*** @jl_pgcstack, align 8
  store %jl_value_t* null, %jl_value_t** %3, align 8
  %6 = call %jl_value_t* @jl_box_int64(i64 signext %1)
  store %jl_value_t* %6, %jl_value_t** %3, align 8
  %7 = bitcast %jl_value_t* %6 to i64*
  %8 = load i64* %7, align 16
  %9 = sitofp i64 %8 to double
  %10 = call %jl_value_t* @jl_gc_alloc_1w()
  %11 = getelementptr inbounds %jl_value_t* %10, i64 -1, i32 0
  store %jl_value_t* inttoptr (i64 140313949825456 to %jl_value_t*), %jl_value_t** %11, align 8
  %12 = bitcast %jl_value_t* %10 to double*
  store double %9, double* %12, align 8
  store %jl_value_t* %10, %jl_value_t** %3, align 8
  call void inttoptr (i64 140322620751200 to void (%jl_value_t*, i64)*)(%jl_value_t* %0, i64 1)
  %13 = getelementptr inbounds %jl_value_t* %0, i64 1
  %14 = bitcast %jl_value_t* %13 to i64*
  %15 = load i64* %14, align 8
  %uadd = call { i64, i1 } @llvm.uadd.with.overflow.i64(i64 %15, i64 -1)
  %16 = extractvalue { i64, i1 } %uadd, 1
  br i1 %16, label %idxend, label %oob

oob:                                              ; preds = %top
  %17 = alloca i64, align 8
  store i64 %15, i64* %17, align 8
  call void @jl_bounds_error_ints(%jl_value_t* %0, i64* %17, i64 1)
  unreachable

idxend:                                           ; preds = %top
  %18 = extractvalue { i64, i1 } %uadd, 0
  %19 = load %jl_value_t** %3, align 8
  %20 = bitcast %jl_value_t* %0 to i8**
  %21 = load i8** %20, align 8
  %22 = bitcast %jl_value_t* %19 to double*
  %23 = load double* %22, align 16
  %24 = bitcast i8* %21 to double*
  %25 = getelementptr double* %24, i64 %18
  store double %23, double* %25, align 8
  %26 = load %jl_value_t** %4, align 8
  %27 = getelementptr inbounds %jl_value_t* %26, i64 0, i32 0
  store %jl_value_t** %27, %jl_value_t*** @jl_pgcstack, align 8
  ret %jl_value_t* %0
}

After:

julia> @code_llvm push!([1.0],2)

define %jl_value_t* @"julia_push!_22628"(%jl_value_t*, i64) {
top:
  call void inttoptr (i64 139720669437280 to void (%jl_value_t*, i64)*)(%jl_value_t* %0, i64 1)
  %2 = getelementptr inbounds %jl_value_t* %0, i64 1
  %3 = bitcast %jl_value_t* %2 to i64*
  %4 = load i64* %3, align 8
  %uadd = call { i64, i1 } @llvm.uadd.with.overflow.i64(i64 %4, i64 -1)
  %5 = extractvalue { i64, i1 } %uadd, 1
  br i1 %5, label %idxend, label %oob

oob:                                              ; preds = %top
  %6 = alloca i64, align 8
  store i64 %4, i64* %6, align 8
  call void @jl_bounds_error_ints(%jl_value_t* %0, i64* %6, i64 1)
  unreachable

idxend:                                           ; preds = %top
  %7 = extractvalue { i64, i1 } %uadd, 0
  %8 = sitofp i64 %1 to double
  %9 = bitcast %jl_value_t* %0 to i8**
  %10 = load i8** %9, align 8
  %11 = bitcast i8* %10 to double*
  %12 = getelementptr double* %11, i64 %7
  store double %8, double* %12, align 8
  ret %jl_value_t* %0
}

[timholy]

Amazing we never caught this before---that seems to have been there since 2011! I guess it's because folks expect push! to be allocating, so no one got worried.

Nice work.

[tkelman]

it makes me a little sad that this makes a difference. one more thing to be vigilant of for perf sensitive code.

[timholy]

Oh, and of course in most cases the value is probably already of type T. That helps explain it.

[mlubin]

Yeah, it's probably not too common. This is only a bottleneck if push! doesn't allocate and the type doesn't match.

[toivoh]

Yes, why did the old formulation trip up type inference? I thought it was based on the data flow.

[KristofferC]

Because the variable item changes from one type to another.

[toivoh]

I always imagined that types were inferred separately for a variable at each point in the code. Ok, good to know.

[StefanKarpinski]

Yeah, it's an obvious optimization that Julia doesn't do yet. It's kind of amazing how far we get on what we do optimize.

[simonster]

It doesn't really trip up type inference (@code_warntype shows that TI knows the type of the variable at every use, and there are no calls to jl_apply_generic) but a variable that changes type still gets boxed.

[toivoh]

Ok, thanks for the clarifications!