Patch: Remove gcc dependency in definition of inlinefunctions
in [PgSql]
|
From: Peter Eisentraut on 17 Jan 2010 14:27 On ons, 2009-12-16 at 10:49 -0500, Tom Lane wrote: > I think you're way overthinking this. Where we started was just > a proposal to try to expand the set of inline-ing compilers beyond > "gcc only". I don't see why we need to do anything but that. The > code is fine as-is except for the control #ifdefs. I have found an Autoconf macro that checks whether the compiler properly supports C99 inline semantics. This would allow us to replace the __GNUC__ conditional with HAVE_C99_INLINE, in this case. Interestingly, however, this check results in GCC <=4.2 *not* supporting C99 inline, apparently because it produces redundant copies of static inline functions. But GCC 4.2 is currently Debian stable, for example, so de-supporting that is probably not yet an option. So, I'm not sure if this would actually solve anyone's problem, but it does call into question that exact semantics that we are looking for. Maybe we just replace __GNUC__ by __GNUC__ || __SOMETHING_ELSE_CC__. Patch attached for entertainment.
From: Peter Eisentraut on 18 Jan 2010 16:20 On sön, 2009-12-06 at 02:21 -0800, Kurt Harriman wrote: > > Which ones does it actually offer any benefit for? > > MSVC is one. I seem to recall that somewhere else it was said that MSVC produces warnings on unused static inline functions. Am I mistaken? Also, if this is mainly for the benefit of MSVC, we don't really need a configure check, do we? -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Kurt Harriman on 18 Jan 2010 19:34 On 1/18/2010 1:20 PM, Peter Eisentraut wrote: > I seem to recall that somewhere else it was said that MSVC produces > warnings on unused static inline functions. Am I mistaken? MSVC does warn about unused static inline functions. The warning is prevented by using __forceinline instead of __inline. > Also, if this is mainly for the benefit of MSVC, we don't really need a > configure check, do we? A configure check seems worthwhile regardless of MSVC. No doubt other compilers, yet unknown, will be discovered to spew alarm upon unreferenced static inline functions. To address the difficulty of testing innumerable present and future platforms, it seems there are three alternatives: a) Wait for complaints, then craft compiler-specific patches and backpatches. b) Keep the existing compiler-specific logic to enable inlining for gcc alone. Craft additional compiler-specific logic for each additional trusted and sufficiently important compiler. c) Use configure to automate the testing of each build environment in situ. The third alternative adapts to new or little-known platforms with little or no manual intervention. It factors platform dependencies out of the code, centralizing them into the build system. It is true that configure doesn't need to test for MSVC's __forceinline keyword. I included that mainly as a placeholder for the benefit of future hackers: likely someone will discover a need for a special keyword to suppress another compiler's warnings. With __forceinine as an example, and the looping logic already in place, it's easy to add an alternative keyword to the list without having to become an autoconf expert. It doesn't make configure slower, since gcc succeeds on the first iteration without trying the __forceinline alternative. Regards, .... kurt -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Kurt Harriman on 18 Jan 2010 22:40 On 1/18/2010 4:42 PM, Tom Lane wrote: > Kurt Harriman<harriman(a)acm.org> writes: >> c) Use configure to automate the testing of each build environment >> in situ. > >> The third alternative adapts to new or little-known platforms >> with little or no manual intervention. > > This argument is bogus unless you can demonstrate a working configure > probe for the property in question. The question about this patch, > from day one, has been whether we have a working configure test. It does work to detect almost exactly the desired property: the absence of a compiler or linker warning when a static inline function is defined but not referenced. "Almost" exactly, because rather than absence of errors or warnings, the exact condition tested is: successful exit code and nothing whatever written to stdout or stderr. (That is a built-in feature of autoconf.) Its success is easily demonstrated on any platform by editing the code within AC_LINK_PROGRAM to induce a compiler or linker warning. >> It is true that configure doesn't need to test for MSVC's >> __forceinline keyword. I included that mainly as a placeholder >> for the benefit of future hackers: likely someone will >> discover a need for a special keyword to suppress another >> compiler's warnings. > > I think including MSVC in the set of compilers targeted by a configure > test is just a waste of code. It's more likely to confuse people than > help them. There's a loop for trying a series of compiler-specific formulas. The list contains two items: (1) "inline" or equivalent as determined by AC_C_INLINE; (2) __forceinline. In a 2-item list, it is easy to see the syntax for adding a third item: in this case, the language is 'sh' and the list is space-separated. The code is: for pgac_kw in "$ac_cv_c_inline" "__forceinline" ; do AC_LINK_IFELSE([AC_LANG_PROGRAM([static $pgac_kw int fun () {return 0;}],[])], [pgac_cv_c_hushinline=$pgac_kw]) test "$pgac_cv_c_hushinline" != no && break done MSVC __forceinline could be dropped from the list: for pgac_kw in "$ac_cv_c_inline" ; do AC_LINK_IFELSE([AC_LANG_PROGRAM([static $pgac_kw int fun () {return 0;}],[])], [pgac_cv_c_hushinline=$pgac_kw]) test "$pgac_cv_c_hushinline" != no && break done Since the list would then have only one element, the whole loop could be dropped. AC_LINK_IFELSE([AC_LANG_PROGRAM([static $pgac_kw int fun () {return 0;}],[])], [pgac_cv_c_hushinline=$pgac_kw]) Someone could reinstate the loop if it is discovered that MSVC is not the only compiler which needs something special to silence its unused-function warning. Regards, .... kurt -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Peter Eisentraut on 19 Jan 2010 02:48
On mån, 2010-01-18 at 16:34 -0800, Kurt Harriman wrote: > MSVC does warn about unused static inline functions. The warning > is prevented by using __forceinline instead of __inline. Hmm, but forceinline is not the same as inline. Are we confident that forcing inlining is not going to lead to disadvantages? Has this been measured? Is there not a setting to disable this particular warning. I read that MSVC has various ways to set that sort of thing. -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |