"Paul Koning" <paulkon...@comcast.net> 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 <stefan.kant...@nexgo.de> 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;
> }

Reply via email to