"Paul Koning" <[email protected]> wrote:
> Yes, that's a rather nasty cut & paste error I made.
I suspected that.
Replacing
!(den & (1L<<31))
with
(signed short) den >= 0
avoids this type of error: there's no need for a constant here!
JFTR: of course the 1L should be just a 1, without suffix.
> But if the 31 is changed to a 15, is the code correct?
> I would think so.
Almost. It's the standard algorithm, and it's correct except
for den == 0, where the current implementation returns 0 as
quotient or the numerator as remainder, while my fix yields an
endless loop (as could be expected for "undefined behaviour").
To avoid that, insert
if (den == 0)
...; // raise the appropriate SIG... here, for example.
between the 2 if's I added.
> For optimization I'd think that an assembly language
> version would make more sense, and a few targets do that.
Moving 2 of 3 conditions from the loop is not an optimisation,
but a necessity!
In other words: why test 3 conditions in every pass of the
loop when you need to test only 1 condition inside the loop,
and the other 2 outside/before the loop?
regards
Stefan
> On Dec 4, 2018, at 5:51 PM, Stefan Kanthak <[email protected]> wrote:
>
> Hi @ll,
>
> libgcc's divmodhi4() function has an obvious bug; additionally
> it shows rather poor inperformant code: two of the three conditions
> tested in the first loop should clearly moved outside the loop!
>
> divmodsi4() shows this inperformant code too!
>
> regards
> Stefan Kanthak
>
> --- divmodhi4.c ---
>
> unsigned short
> __udivmodhi4(unsigned short num, unsigned short den, int modwanted)
> {
> unsigned short bit = 1;
> unsigned short res = 0;
>
> #ifdef BUGFIX
> if (den > num)
> return modwanted ? num : 0;
> if (den == num)
> return modwanted ? 0 : 1;
> while ((signed short) den >= 0)
> #else // original, buggy and inperformant code
> while (den < num && bit && !(den & (1L<<31))) // unsigned shorts are 16 bit!
> #endif
> {
> den <<=1;
> bit <<=1;
> }
> while (bit)
> {
> if (num >= den)
> {
> num -= den;
> res |= bit;
> }
> bit >>=1;
> den >>=1;
> }
> if (modwanted) return num;
> return res;
> }
