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?  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.

> 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,
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 :-)


Segher

Reply via email to