JuliaLang/julia

Possible improvements to DimensionMismatch errors

Open

#33,273 opened on Sep 16, 2019

View on GitHub
 (3 comments) (3 reactions) (0 assignees)Julia (5,773 forks)batch import
good first issue

Repository metrics

Stars
 (48,709 stars)
PR merge metrics
 (Avg merge 20d 6h) (157 merged PRs in 30d)

Description

There are 14 hardcoded expressions like:

throw(DimensionMismatch("dimensions must match"))

in the code base. Also, as shown in

julia> x + y
ERROR: DimensionMismatch("dimensions must match")
...

this doesn't print nicely as an error (there is some ´(...)` syntax markers) and it doesn't say why the dimensions doesn't match.


There are also some kinda bad strings given to DimensionMismatch like:

throw(DimensionMismatch("transpose"))

in transpose.jl in LinearAlgebra.


In some cases, it is possible that allocating the string inside the functions for throwing the error can lead to a bunch of code bloat due to GC frames:

julia> @code_llvm debuginfo=:none promote_shape((1,), (2,))

define [1 x i64] @julia_promote_shape_15929([1 x i64] addrspace(11)* nocapture nonnull readonly dereferenceable(8), [1 x i64] addrspace(11)* nocapture nonnull readonly dereferenceable(8)) {
top:
  %gcframe = alloca %jl_value_t addrspace(10)*, i32 3
  %2 = bitcast %jl_value_t addrspace(10)** %gcframe to i8*
  call void @llvm.memset.p0i8.i32(i8* %2, i8 0, i32 24, i32 0, i1 false)
  %3 = call %jl_value_t*** inttoptr (i64 4435471600 to %jl_value_t*** ()*)() #5
  %4 = getelementptr %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)** %gcframe, i32 0
  %5 = bitcast %jl_value_t addrspace(10)** %4 to i64*
  store i64 2, i64* %5
  %6 = getelementptr %jl_value_t**, %jl_value_t*** %3, i32 0
  %7 = load %jl_value_t**, %jl_value_t*** %6
  %8 = getelementptr %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)** %gcframe, i32 1
  %9 = bitcast %jl_value_t addrspace(10)** %8 to %jl_value_t***
  store %jl_value_t** %7, %jl_value_t*** %9
  %10 = bitcast %jl_value_t*** %6 to %jl_value_t addrspace(10)***
  store %jl_value_t addrspace(10)** %gcframe, %jl_value_t addrspace(10)*** %10
  %11 = getelementptr [1 x i64], [1 x i64] addrspace(11)* %0, i64 0, i64 0
  %12 = getelementptr [1 x i64], [1 x i64] addrspace(11)* %1, i64 0, i64 0
  %13 = load i64, i64 addrspace(11)* %11, align 8
  %14 = load i64, i64 addrspace(11)* %12, align 8
  %15 = icmp eq i64 %13, %14
  br i1 %15, label %L9, label %L6

L6:                                               ; preds = %top
  %16 = bitcast %jl_value_t*** %3 to i8*
  %17 = call noalias nonnull %jl_value_t addrspace(10)* @jl_gc_pool_alloc(i8* %16, i32 1624, i32 16) #1
  %18 = bitcast %jl_value_t addrspace(10)* %17 to %jl_value_t addrspace(10)* addrspace(10)*
  %19 = getelementptr %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)* addrspace(10)* %18, i64 -1
  store %jl_value_t addrspace(10)* addrspacecast (%jl_value_t* inttoptr (i64 4539098032 to %jl_value_t*) to %jl_value_t addrspace(10)*), %jl_value_t addrspace(10)* addrspace(10)* %19
  %20 = bitcast %jl_value_t addrspace(10)* %17 to %jl_value_t addrspace(10)* addrspace(10)*
  store %jl_value_t addrspace(10)* addrspacecast (%jl_value_t* inttoptr (i64 4554031888 to %jl_value_t*) to %jl_value_t addrspace(10)*), %jl_value_t addrspace(10)* addrspace(10)* %20, align 8
  %21 = addrspacecast %jl_value_t addrspace(10)* %17 to %jl_value_t addrspace(12)*
  %22 = getelementptr %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)** %gcframe, i32 2
  store %jl_value_t addrspace(10)* %17, %jl_value_t addrspace(10)** %22
  call void @jl_throw(%jl_value_t addrspace(12)* %21)
  unreachable

L9:                                               ; preds = %top
  %23 = insertvalue [1 x i64] undef, i64 %13, 0
  %24 = getelementptr %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)** %gcframe, i32 1
  %25 = load %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)** %24
  %26 = getelementptr %jl_value_t**, %jl_value_t*** %3, i32 0
  %27 = bitcast %jl_value_t*** %26 to %jl_value_t addrspace(10)**
  store %jl_value_t addrspace(10)* %25, %jl_value_t addrspace(10)** %27
  ret [1 x i64] %23
}

While using something like

@noinline throw_dimensionmismatch() = throw(DimensionMismatch("dimensions must match"))

gives a nicer

julia> @code_llvm debuginfo=:none promote_shape((1,), (2,))

define [1 x i64] @julia_promote_shape_17082([1 x i64] addrspace(11)* nocapture nonnull readonly dereferenceable(8), [1 x i64] addrspace(11)* nocapture nonnull readonly dereferenceable(8)) {
top:
  %2 = getelementptr [1 x i64], [1 x i64] addrspace(11)* %0, i64 0, i64 0
  %3 = getelementptr [1 x i64], [1 x i64] addrspace(11)* %1, i64 0, i64 0
  %4 = load i64, i64 addrspace(11)* %2, align 8
  %5 = load i64, i64 addrspace(11)* %3, align 8
  %6 = icmp eq i64 %4, %5
  br i1 %6, label %L8, label %L6

L6:                                               ; preds = %top
  call void @julia_throw_dimensionmismatch_17029()
  call void @llvm.trap()
  unreachable

L8:                                               ; preds = %top
  %7 = insertvalue [1 x i64] undef, i64 %4, 0
  ret [1 x i64] %7
}

This might not be a problem in practice though but promote_shape might be called in quite tight places so maybe.

Contributor guide