Re: [ft-devel] [patch] clean up MulDiv in bytecode interpreter

[Werner LEMBERG]

Hello Alexei!


> 1) TT_MULDIV macros are replaced with to direct calls to FT_MulDiv
> because they do not do anything useful

Good idea.  Please commit this (as a separate patch).

> 2) TT_MulFix14 is now a macro that uses FT_MulFix, no need to
> duplicate the code

Please be very careful here.  David has explicitly introduced this
function *after* the standard FT_MulDiv functions, so there must be a
reason (which unfortunately is not documented in the ChangeLog).  Note
that this function is used extremely frequently in bytecode, so maybe
it contains subtle optimizations not easily recognizable; David is
very good in handling this, he even compared assembler output of
various compilers to the optimize C code.

My gut feeling is to not change the code.  It's just a few bytes more.

However, I like the introduction of the FT_DivFix14 macro.

> 3) FT_MulFix and FT_DivFix are called when they are meant to be
> called, rather than using 0x10000L explicitly

Looks good.  However, please provide a separate patch for easier
review.

> 4) scaling by 64 is hardly an easy overflow trigger, so we can risk
> direct arithmetic, IMHO

Well, I think you are wrong.  Besides this, using direct arithmetics
hides the `no round' issue.  So I rather dislike this change.


    Werner