"Segher Boessenkool" <seg...@kernel.crashing.org> wrote: > On Wed, Dec 05, 2018 at 02:19:14AM +0100, Stefan Kanthak wrote: >> "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. > > "int" can be 16 bits only, I think?
This function works on short int, which can only be 16 bits. > If you change to 15 you can remove the L, sure. > >> > But if the 31 is changed to a 15, is the code correct? >> > I would think so. > > I think so, too. > >> > 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! > > The compiler can optimise things quite well. It but doesn't, see <https://godbolt.org/z/e3CUY4>: lines 15 to 20 are inside the loop, line 6 and 9 to 11 skip the loop. >> 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? > > Maybe the code is easier to read this way. Maybe it doesn't matter for > performance. Maybe no one cared, this routine is for correctness anyway, No, no & yes, yes: it's not very likely that it will really be called. OTOH, an unsuspecting kid may find and copy it, assuming that a routine shipped with GCC can't be THAT bad.-P Or the (almost) identical <https://github.com/gcc-mirror/gcc/blob/master/libgcc/udivmodsi4.c> ... > any target that cares for performance will do an asm version, or (partly) > inline this even. > > Send a patch if you want to see it changed :-) I saw my last PDP-11 more than 30 years ago! regards Stefan
