codegen: warn on unbound static parameter

[t-bltg]

This is a proposal which fixes #29393, specifically by adding a warning as proposed in #29393 (comment).

With this PR applied, some of the following examples collected from the linked issues of #29393 are throwing a warning (format to be discussed ?) when unnecessary boxing occurs because of some unbound static parameter:

unbound.jl

using InteractiveUtils

f1(y::T) where {T} = 2y
f2(y::T) where {N, T} = 2y  # 29393
f3(x::Union{Vector{T}, NTuple{N, T}}) where {N, T} = x[1]  # 40681
f4(::Type{T}, x) where {T} = 2x  # 33847
f5(x::Int) where {T} = 2x  # 35935, 45846

@code_llvm f1(1)  # good
@code_llvm f2(1)  # bad
@code_llvm f3([1])  # bad
@code_llvm f3((1, 2))  # good
@code_llvm f4(Bool, 1)  # good
@code_llvm f5(1)  # bad

Output running $ julia unbound.jl

;  @ [...]/unbound.jl:3 within `f1`
define i64 @julia_f1_236(i64 signext %0) #0 {
top:
; ┌ @ int.jl:88 within `*`
   %1 = shl i64 %0, 1
; └
  ret i64 %1
}

WARNING: [Symbol("[...]/unbound.jl"):4] in method :f2, static parameter :N is unbound
;  @ [...]/unbound.jl:4 within `f2`
define nonnull {}* @japi3_f2_269({}* %0, {}** %1, i32 %2, {}** %3) #0 {
top:
  %4 = alloca {}**, align 8
  store volatile {}** %1, {}*** %4, align 8
  %5 = bitcast {}** %1 to i64**
  %6 = load i64*, i64** %5, align 8
; ┌ @ int.jl:88 within `*`
   %7 = load i64, i64* %6, align 8
   %8 = shl i64 %7, 1
; └
  %9 = call nonnull {}* @jl_box_int64(i64 signext %8)
  ret {}* %9
}

WARNING: [Symbol("[...]/unbound.jl"):5] in method :f3, static parameter :N is unbound
;  @ [...]/unbound.jl:5 within `f3`
define nonnull {}* @japi3_f3_270({}* %0, {}** %1, i32 %2, {}** %3) #0 {
top:
  %4 = alloca {}**, align 8
  store volatile {}** %1, {}*** %4, align 8
  %5 = load {}*, {}** %1, align 8
; ┌ @ array.jl:861 within `getindex`
   %6 = bitcast {}* %5 to { i8*, i64, i16, i16, i32 }*
   %7 = getelementptr inbounds { i8*, i64, i16, i16, i32 }, { i8*, i64, i16, i16, i32 }* %6, i64 0, i32 1
   %8 = load i64, i64* %7, align 8
   %.not = icmp eq i64 %8, 0
   br i1 %.not, label %oob, label %idxend

oob:                                              ; preds = %top
   %9 = alloca i64, align 8
   store i64 1, i64* %9, align 8
   call void @jl_bounds_error_ints({}* %5, i64* nonnull %9, i64 1)
   unreachable

idxend:                                           ; preds = %top
   %10 = bitcast {}* %5 to i64**
   %11 = load i64*, i64** %10, align 8
   %12 = load i64, i64* %11, align 8
; └
  %13 = call nonnull {}* @jl_box_int64(i64 signext %12)
  ret {}* %13
}

;  @ [...]/unbound.jl:5 within `f3`
define i64 @julia_f3_271([2 x i64]* nocapture nonnull readonly align 8 dereferenceable(16) %0) #0 {
top:
; ┌ @ tuple.jl:29 within `getindex`
   %1 = getelementptr inbounds [2 x i64], [2 x i64]* %0, i64 0, i64 0
; └
  %2 = load i64, i64* %1, align 8
  ret i64 %2
}

;  @ [...]/unbound.jl:6 within `f4`
define i64 @julia_f4_274(i64 signext %0) #0 {
top:
; ┌ @ int.jl:88 within `*`
   %1 = shl i64 %0, 1
; └
  ret i64 %1
}

WARNING: [Symbol("[...]/unbound.jl"):7] in method :f5, static parameter :T is unbound
;  @ [...]/unbound.jl:7 within `f5`
define nonnull {}* @japi3_f5_276({}* %0, {}** %1, i32 %2, {}** %3) #0 {
top:
  %4 = alloca {}**, align 8
  store volatile {}** %1, {}*** %4, align 8
  %5 = bitcast {}** %1 to i64**
  %6 = load i64*, i64** %5, align 8
; ┌ @ int.jl:88 within `*`
   %7 = load i64, i64* %6, align 8
   %8 = shl i64 %7, 1
; └
  %9 = call nonnull {}* @jl_box_int64(i64 signext %8)
  ret {}* %9
}

While working on this PR, I modified a few things in codegen.cpp and jltypes.c (I thought it wasn't worth making a separate PR): I'd be happy to split that to another PR if I was wrong.

I'm unsure how to write corresponding tests for these new warnings in CI (if needed), so any advice is welcome. From the warnings thrown by the test suite, there doesn't seem to be false positives at first glance.

[JeffBezanson]

This isn't the right place to implement the warning (if we want one); it should be in the front end when the method is defined.

[fredrikekre]

Linter warns for f2 and f5 already. Could maybe be improved to cover more cases though.

[jakobnissen]

I think a warning would be nice, even if linters could catch this reliably. I can't come up with an example where unbound typevars are intended - and they are an obscure performance concern.

[t-bltg]

@JeffBezanson, could you point me to a more precise source location (when you speak about the frontend), where throwing a warning would be acceptable ?

[t-bltg]

Superseded by #46608.