Re: [patches] [PATCH] Make libm smaller using wrappers

["Joseph S. Myers" <joseph@xxxxxxxxxxxxxxxx>]

On Wed, 28 Oct 2009, Maxim Kuvyrkov wrote:

> @@ -645,19 +652,19 @@ sub get_ulps {
>      # Return 0 instead of BUILD_COMPLEX (0,0)
>      if (!exists $results{$test}{'real'}{'ulp'}{$float} &&
>  	!exists $results{$test}{'imag'}{'ulp'}{$float}) {
> -      return "0";
> +      return "BUILD_COMPLEX ($default_ulp, $default_ulp)";

Note the comment saying it is deliberately doing something that it is no 
longer doing after your patch.  How is the result used, and why is this 
change OK?  Certainly the comment will need removing or changing if the 
output change stays.

> diff --git a/math/test-dbl-wrap.c b/math/test-dbl-wrap.c
> new file mode 100644
> index 0000000..c5dfe2d
> --- /dev/null
> +++ b/math/test-dbl-wrap.c
> @@ -0,0 +1,108 @@
> +#include <math.h>
> +#include <stdio.h>
> +
> +#define N 4
> +
> +int
> +main (void)
> +{

All new tests should be using the test-skeleton infrastructure if 
possible.

Otherwise the patch seems OK, though it would be good to have specific 
guidance somewhere (a mailing list message would be OK) on how to 
determine what changes are needed to keep this option group working 
correctly if a new libm function is added.  Do I understand correctly that 
no change is strictly needed for any given function - but if no change is 
made all three versions of the new function will be present whatever the 
setting for this option group?  If so, the documentation would be saying 
how to make the option group effective for a new function that is making a 
significant contribution to libm size.  (It might of course also be useful 
for any new functions to get an associated option group to disable a 
family of functions.)

-- 
Joseph S. Myers
joseph@xxxxxxxxxxxxxxxx