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

2012-12-04 Thread Alexei Podtelezhnikov 
Werner,

As it stands right now, ttinterp.c uses multiple forms of
multiplication-division calls: macro TT_MULDIV, direct FT_MulDiv, and
custom TT_MulFix14. This is confusing and unjustified IMHO, so I wanted to
consolidate and harmonize those calls. The attached patch mostly does the
following

1) TT_MULDIV macros are replaced with to direct calls to FT_MulDiv because
they do not do anything useful
2) TT_MulFix14 is now a macro that uses FT_MulFix, no need to duplicate the
code
3) FT_MulFix and FT_DivFix are called when they are meant to be called,
rather than using 0x1L explicitly
4) scaling by 64 is hardly an easy overflow trigger, so we can risk direct
arithmetic, IMHO

I have done some limited testing. Objections?

Alexei


ft-ttinterp.patch
Description: Binary data
___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel

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

2012-12-04 Thread 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 0x1L 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

___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel