[ShriramShastry]

fixpoint math log() and log10() function. Logarithm
of x is the power to which e/10 would have to be raised
to equal x.

Implementation of log10(x) is a derivative of log2(x)/log2(10)
and loge(x) as log2(x)/log2(e)

Signed-off-by: Shriram Shastry malladi.sastry@intel.com

[lgirdwood]

@singalsu fyi

[cujomalainey]

Definitely getting cleaner :) still appears you have duplication issues

[ShriramShastry]

@ShriramShastry can you check CI, looks like we could have a build failure. Also please update the review comments above and resolve all the comments that are done.

Done. Resolved CI and PR comments

[lgirdwood]

@singalsu any comments.

[singalsu]

Some comments improvement needed.

[singalsu]

Thanks, only a minor change proposed, easier two macros names and move Q-format note to comment.

[cujomalainey]

Marc is correct, we should not be discussing implementation here

[cujomalainey]

s/below/above

[cujomalainey]

You still need to add the inline keyword to get the optimization

[marc-hb]

I compared gcc and clang outputs a number of times and I've never seen inline making any difference, at least never with -On where n > 0. The optimizer always thinks it knows better. That's why there is non-portable stuff like ALWAYS_INLINE.

(static inline is still useful in .h files to silence some warnings)

[cujomalainey]

given we know xcc isnt the smartest sometimes i think it is still wise to have the hints for it

[ShriramShastry]

if I change to inline ln_int32() and inline log10_int32(), I could see below error

No codespell typos will be found - file '/usr/share/codespell/dictionary.txt': No such file or directory
ERROR: inline keyword should sit between storage class and type
#10: FILE: src/math/log_10.c:28:
+uint32_t inline log10_int32(uint32_t numerator)

ERROR: inline keyword should sit between storage class and type
#23: FILE: src/math/log_e.c:29:
+uint32_t inline ln_int32(uint32_t numerator)

In case I make changes in log.h like below

static inline uint32_t ln_int32(uint32_t numerator)
{
	return((uint32_t)Q_SHIFT_RND((int64_t)base2_logarithm(numerator) *
				     ONE_OVER_LOG2_E, 64, 32));
}
static inline uint32_t log10_int32(uint32_t numerator)
{
		return((uint32_t)Q_SHIFT_RND((int64_t)base2_logarithm(numerator) *
				     ONE_OVER_LOG2_10, 63, 32));
}

then log10_int32(uint32_t numerator) and ln_int32(uint32_t numerator) in log_10.c and log_e.c will be redundant, the impact of removing log_10.c and log_e.c will result in error in cmocka compilation. cmocka CmakeList.txt is using of log_10.c and log_e.c files.

cmocka_test(base_10_logarithm
	base_10_logarithm.c
	${PROJECT_SOURCE_DIR}/src/math/log_10.c
	${PROJECT_SOURCE_DIR}/src/math/base2log.c
)

cmocka_test(base_e_logarithm
	base_e_logarithm.c
	${PROJECT_SOURCE_DIR}/src/math/log_e.c
	${PROJECT_SOURCE_DIR}/src/math/base2log.c
)

[lgirdwood]

@ShriramShastry do you think you can fix review comment in the next week or two ? I will need to start preparing for v2.1 tag soon.

[ShriramShastry]

@ShriramShastry do you think you can fix review comment in the next week or two ? I will need to start preparing for v2.1 tag soon.

Yes, I think so.

[lgirdwood]

@cujomalainey good for you ?