CCTK_CHECK_C_INLINE can define away the inline keyword

Comments (13)

1. 

   Erik Schnetter

   • removed comment

   GCC and many other compilers supported the inline keyword for a long time. Unfortunately, ANSI C now standardised this in an incompatible manner, generating essentially two incompatible dialects of C. In Cactus, we need to decide which of these dialects we want to support. Historically, we of course supported the inofficial one (since it was the only one that existed).

   I tried the C99 dialect some time ago (maybe one year ago?), and things failed. I tested more thorns that just the standard ET thorns; in particular, I included many external libraries as well.

   Many compilers have options to switch behaviour; in other cases, we can use autoconf. For example, gcc 4.6 (I believe) supports both dialects, depending on the -std setting. IBM's xlc treats the keywords inline and inline differently... PGI may have something similar.

   Yes, this line 180 is wrong -- it should test HAVE_CCTK_C_INLINE instead.

   • 2011-08-03T17:30:28+00:00
2. 

   Roland Haas reporter

   • removed comment

   Hmm, if #defining inline in cctk_Config.h helped when using (I assume by including their header files) external libraries then was this related to the different meaning of "extern inline" in C99 and gcc (classic) as in http://gcc.gnu.org/gcc-4.3/porting_to.html ?

   For gcc 4.6 I currently add -fgnu89-inline to CFLAGS. Otherwise I get errors about strsep_3c and similar things in the string2.h header (which is part of gcc 4.6.1) unless I turn off the inline #define.

   Both -std=c99 and -std=gnu99 now (gcc 4.6) use the C99 ANSI behaviour (see http://gcc.gnu.org/onlinedocs/gcc/Inline.html).

   • 2011-08-04T10:31:52+00:00
3. 

   Erik Schnetter

   • removed comment

   Ah, that's where these errors were coming from... I think I "remedied" them by allowing duplicate symbols in the linker.

   In this case, I think that adding -fgnu89-inline to the Cactus options is the best short-term solution. In the long term, we may want to switch to the new inline semantics; this would include ensuring that shared C/C++ header files behave correctly, and updating the C Cactus thorns which use inline.

   1. define inline was always part of Cactus; this is standard procedure in GNU autoconf, and was not introduced to correct a C99 problem.

   • 2011-08-04T12:03:45+00:00
4. 

   Roland Haas reporter

   • removed comment

   Well yes, it is part of autoconf's AC_INLINE macro. That macro however checked for any kind of inline keyword and not just a particular one that makes the inline function externally visible (ie. CCTK_CHECK_C_INLINE checks for GNU's inline semantics whereas AC_INLINE checked for any inline keyword). So it would never define away inline even though the compiler does indeed support inline functions :-)

   Any solution that lets me compile is fine with me though.

   I attach a patch for cctk.h and aclocal.m4.

   This still leaves the pgi compiler out in the cold though since I do not know how to make it use the gnu inline semantics. Any help is welcome given that pgi was the compiler I'd originally like to get to work.

   • 2011-08-04T16:17:54+00:00
5. 

   Erik Schnetter

   • removed comment

   I believe that HAVE_CCTK_C_INLINE is already defined when the inline keyword is supported, thus changing aclocal.m4 should not be necessary. On the other hand, the correction of cctk.h seems correct -- please apply.

   • 2011-08-13T22:58:30+00:00
6. 

   Roland Haas reporter

   • removed comment

   HAVE_CCTK_C_INLINE was defined only for the case where one actually had to '''re'''-#define inline (there are three cases [a] "inline" itself uses gnu89 semantics, [b] "inline" can be #defined to something that uses the gnu89 semantics, [c] nothing works), but not for the case where "inline" itself uses the old gnu89 semantics.

   I cannot apply this myself since it involves the flesh and I do not have write permissions for it.

   • 2011-08-14T11:16:24+00:00
7. 

   Erik Schnetter

   • removed comment

   I applied the full patch.

   You said "lets me compile": I do not understand why the previous version should lead to problems with the PGI compiler. Things should have worked as things were. Where a function is inlined or not is always the choice of the compiler, with or without inline keyword. A compiler that doesn't inline on its own isn't worth its money.

   • 2011-08-14T12:34:13+00:00
8. 

   Roland Haas reporter

   • removed comment

   Thank you.

   Hmm, "lets me compile" was not really very clear, yes. Sorry.

   The previous version compiled with the PGI compiler. It #defined "inline" to the empty string. So the only inlining happening was due to the optimizer deciding that a given is a good function to inline. All the inline hints were lost (since, as you correctly pointed out, inlining is always the choice of the compiler, it does not have to do anything at all when it sees an "inline" statement). I would have to run speed tests to check if PGI's compiler is smart enough to inline all the functions we want to be inlined even if all "inline" keywords were removed.

   Using the previous version, gcc 4.6, and no further options I could not compile (actually link) because of the strsep_3c errors (nor can i now I think). I can and could compile/link using gcc 4.6 by either switching the CCTK_C_INLINE check back to the old AC_INLINE check or by using -fgnu89-inline. So right now I can compile and am reasonably happy (on Kraken I will likely use AC_INLINE to keep the "inline" hints present until/if-not I find a way to change PGI's inline semantics to gnu89).

   Compilation seems to fail (or be possible to fail) since the gnu89 and c99 semantics for "inline" differ in how visible "inline" functions are. The issue with gcc 4.6 was that its compiler specific string2.h file checks for the inline flavor used by looking at GNUC_STDC_INLINE. If we redefine inline then this check fails (since GNUC_STDC_INLINE is true but inline actually behaves like gnu89) which causes string2.h to make the inline functions visible to the linker which in turn causes link time errors.

   • 2011-08-14T13:31:41+00:00
9. 

   Erik Schnetter

   • removed comment

   I see; yes, I've seen these string.h problems. I usually circumvent them by allowing multiply defined symbols in the linker, specifically by adding -Wl,--allow-multiple-definition to LDFLAGS on Kraken.

   Another way to circumvent this may be to #include <string.h> before <cctk.h>; this way, the inline keyword is not redefined when the standard headers are read. This may require many code changes, and is also somewhat inelegant since things should not depend on the order in which include files are read.

   On a side note, I highly recommend using the Intel compiler over the PGI compiler. I find it produces faster code and is easier to use.

   • 2011-08-14T15:21:45+00:00
10. 

    Roland Haas reporter

    • removed comment

    Including cctk.h last might not not always be possible. Eg. if one wants to include some header files (unistd.h [see the flesh] or cxxabi.h [the backtrace patches in Carpet/Intialize.cc]) only if they were detected by configure then cctk.h has to come before these header files (not an issue for string.h admittedly).

    I have used the --multiple-definition flag (for other reasons) in the past (-gnu89-inline also fixes this issue). Does it check that the two definitions are actually identical? ie. that they are not two completely different functions whose names clash?

    Anyhow I doubt it is worth spending much time on this until it is decided whether Cactus should adhere to C99 or the traditional behaviour. We know how to compile on each system.

    • 2011-08-14T15:40:16+00:00
11. 

    Roland Haas reporter

    • changed status to resolved
    • removed comment

    • 2011-08-17T08:27:02+00:00
12. 

    Roland Haas reporter

    • edited description
    • changed status to closed

    • 2019-02-21T20:15:11+00:00
13. 

    Roland Haas reporter

    • edited description

    • 2019-03-24T22:57:58+00:00
14. Log in to comment