core_matrix uses suboptimal index type for matrix functions on 4-bit archs
Closed this issue · 1 comments
I.e. in
void matrix_add_const(ee_u32 N, MATDAT *A, MATDAT val) {
ee_u32 i,j;
for (i=0; i<N; i++) {
for (j=0; j<N; j++) {
A[i*N+j] += val;
}
}
}
since unsigned types are allowed to wrap/overflow this code is not the equivalent of some abstract A[i][j] when pointers are 64-bit (where is no guarantee that i*N + j
doesn't wrap). Solution: either use signed type or size_t
.
Note, that it becomes a correctness issue, not a performance one, if N
is allowed to be > 2^16.
@danilaml - Thank you for your feedback. I would argue that changing to signed or size_t
(which is unsigned) doesn't alleviate your concern: the programmer must be aware of the architectural limits for datatypes beforehand (i.e. <limits.h>
). Also CoreMark is a benchmark: the values for N have already been pre-decided and aren't going to change. CoreMark is not intended to be a matrix-math library.