On Thu, 28 Mar 2013, Alexander Ivchenko wrote: > Hi, > > 4.8 is now branched, lets come back to the discussion that we had > before. I updated the patch a little > bit since we now have linux-protos.h and linux-android.c files. > > I tried to preserve the avaiability of c99 for all targets, but it's > pretty difficult, because we are changing > the defaults. Passing an empty string as second argument doesn't look > very good, but on the other hand > the user has one clear way for checking the presence of a certain > function. But of course we can create > another function, that will call targetm.libc_has_function > (function_class, "") within itself.
Observations on this patch version: * You still shouldn't be calling with "" as function name (or 0, in i386.md) unless there's a good reason the relevant function can't readily be identified. (Actually, the TARGET_C99_FUNCTIONS checks in i386.md seem wrong - the instruction patterns expand things inline and the semantics of an isinf insn pattern are nothing to do with whether the target library has C99 functions; C99 doesn't provide isinf as a function anyway, only as a macro.) If you don't yet need the name in any hook implementation, feel free to define the hooks without the name argument and it can be added later if needed. * I don't see how your change for Darwin preserves the existing semantics; you appear to make it use no_c99_libc_has_function, but the existing semantics are that it has C99 functions except for 32-bit powerpc Darwin versions older than 10.3. * I don't see how your change preserves semantics for Solaris either - you add a definition in sol2.h (for Solaris 9) but don't override it in sol2-10.h (for Solaris 10, which has the C99 functions). * I don't see anything to preserve semantics for AIX 4.3 and 5.1 (the AIX versions for which TARGET_C99_FUNCTIONS was not defined). * I don't see anything to preserve semantics for alpha*-dec-*vms*. * I don't see anything to preserve semantics for hppa*-*-hpux*. * I don't see anything to preserve semantics for i[34567]86-pc-msdosdjgpp*. * I don't see anything to preserve semantics for i[34567]86-*-cygwin*, or x86_64-*-cygwin*, or i[34567]86-*-mingw*, or x86_64-*-mingw*, or i[34567]86-*-interix[3-9]*. * It looks rather like microblaze*-*-* don't use elfos.h, so meaning semantics aren't preserved for those (non-Linux) targets either. Now, I don't know if there's a good reason for not using that file (ask the architecture maintainer), but in any case semantics should be preserved. * I don't see anything to preserve semantics for mmix-knuth-mmixware, or pdp11-*-*, or picochip-* (more non-ELF targets). * The patch is missing the poisoning of the removed target macros in system.h. Now, for verifying semantics are unchanged for existing targets, you might want to adapt contrib/config-list.mk to show in some way the TARGET_C99_FUNCTIONS setting resulting from tm.h (before your patch) or the new hook setting (after your patch), so you can compare for all the targets listed there. That's not perfect - the list of targets may not be complete - but if you fix all the issues listed above then before-and-after comparison for lots of targets using config-list.mk would be a good way of gaining confidence in the lack of unintended changes in the patch. -- Joseph S. Myers jos...@codesourcery.com
