On Mon, Aug 3, 2020 at 6:17 PM Crt Mori <cmo@xxxxxxxxxxx> wrote: > > TAdut4 was calculated each iteration although it did not change. In light > of near future additions of the Extended range DSP calculations, this > function refactoring will help reduce unrelated changes in that series as > well as reduce the number of new functions needed. Okay! > Also converted shifts in this function of signed integers to divisions as > that is less implementation-defined behavior. This is what I'm wondering about. Why? ... > - Ha_customer = ((s64)Ha * 1000000LL) >> 14ULL; > - Hb_customer = ((s64)Hb * 100) >> 10ULL; > + Ha_customer = div64_s64((s64)Ha * 1000000LL, 16384); > + Hb_customer = div64_s64((s64)Hb * 100, 1024); Have you checked the code on 32-bit machines? As far as I can see the div64_*64() do not have power of two divisor optimizations. I bet it will generate a bulk of unneeded code. ... > - calcedKsTO = ((s64)((s64)Ga * (prev_object_temp - 25 * 1000LL) > - * 1000LL)) >> 36LL; > - calcedKsTA = ((s64)(Fb * (TAdut - 25 * 1000000LL))) >> 36LL; > - Alpha_corr = div64_s64((((s64)(Fa * 10000000000LL) >> 46LL) > - * Ha_customer), 1000LL); > + calcedKsTO = div64_s64((s64)((s64)Ga * (prev_object_temp - 25 * 1000LL) > + * 1000LL), 68719476736); > + calcedKsTA = div64_s64((s64)(Fb * (TAdut - 25 * 1000000LL)), 68719476736); > + Alpha_corr = div64_s64(div64_s64((s64)(Fa * 10000000000LL), 70368744177664) > + * Ha_customer, 1000LL); This is less readable and full of magic numbers in comparison to the above (however, also full of magics, but at least gives better hint). ... > + TAdut4 = (div64_s64(TAdut, 10000LL) + 27315) * > + (div64_s64(TAdut, 10000LL) + 27315) * > + (div64_s64(TAdut, 10000LL) + 27315) * > + (div64_s64(TAdut, 10000LL) + 27315); Shouldn't you switch to definitions from units.h? (perhaps as a separate change) -- With Best Regards, Andy Shevchenko
