eembc/coremark

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.