"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; > }