Re: [PATCH v2 1/3] Cleanup __ieee754_sqrt(f/l)

[Joseph Myers <joseph at codesourcery dot com>]

On Fri, 22 Dec 2017, Wilco Dijkstra wrote:

> Joseph Myers wrote:
> 
> > float128_private.h isn't an external header.  It includes math.h and other 
> > headers, then redefines symbols from them, and is included before the main 
> > implementation of a float128 function.  Naturally your problem needs 
> > debugging, since of course libm should be exporting sqrtf128.  (And 
> > float128_private.h shouldn't be included at all when the type-generic 
> > wrappers are, so shouldn't have any opportunity to cause trouble with the 
> > automatically generated w_sqrtf128 which is what ought to be defining 
> > sqrtf128 as an alias to __sqrtf128.)
> 
> Well it seems the issue is all the autogenerated magic templates. It appears
> they are used to both define simple wrappers as well as complex float128
> functions which happen to use sqrt (in the form of M_SQRT). For the former
> you want to disable the asm redirect, for the latter you must redirect it. Given
> they are all generated in the same way, it's impossible to do the right thing.
> So I don't think we can change M_SQRT until the wrapper magic has been
> removed or at least greatly simplified.

"impossible to do the right thing" does not make sense here.  Whether 
redirections should be disabled is a function of the particular template 
in question, and so should be handled through the templates that want to 
disable redirection defining NO_MATH_REDIRECT and those that don't not 
defining it.  Defining it globally in the Makefile as this patch version 
does is obviously wrong.

Is your problem actually that math-type-macros-float128.h, unlike the 
other math-type-macros-*.h files, includes <math.h> and <complex.h>, 
meaning they get included before any code from the template has a chance 
to define NO_MATH_REDIRECT?  If so, I'd suggest removing those includes 
and seeing if that helps.

Now, maybe it's not enough in all cases: math-type-macros-double.h 
includes libm-alias-double.h, of which the ldbl-opt version includes 
math_ldbl_opt.h which includes math.h, and likewise for ldouble.  So it 
would also be important to look at sqrt / sqrtl wrappers in *static* libm 
for ldbl-opt configurations (as for most configurations, only static libm 
is using the type-generic wrappers rather than the compat ones - but we 
don't have good testsuite coverage for whether static libraries provide 
the intended API, so missing symbols in them could easily not result in 
any test failures).

I'm not sure anything ought to be depending on math_ldbl_opt.h including 
math.h and math_private.h either; quite possibly those includes could also 
be safely removed, maybe adding explicit includes to any source files that 
were relying on them (though at that point I think you're in the territory 
of wanting before-and-after comparison of stripped installed shared 
libraries from build-many-glibcs.py --strip, to make sure there are no 
changes to installed libraries at all).

But while I think such include removals are a sensible cleanup, relying on 
not having any indirect inclusion path to math.h from math-type-macros-*.h 
is also fragile, and I don't think it's necessary.  Rather, you could have 
a mechanism for templates to cause NO_MATH_REDIRECT to be defined before 
the automatic inclusion of math-type-macros-*.h.  For example, have the 
math/Makefile template logic generate such a define for functions in 
a makefile variable $(gen-calls-no-math-redirect).  If you do something 
like that, you don't need to remove any includes.  And you also avoid the 
clear incorrectness of defining NO_MATH_REDIRECT for all templates rather 
than only particular cases needing it.

(Once you have such makefile logic of course you no longer define 
NO_MATH_REDIRECT directly in w_sqrt_template.c.)

-- 
Joseph S. Myers
joseph@codesourcery.com