[Einstein Toolkit] #484: CCTK_CHECK_C_INLINE can define away the inline keyword

List overview All Threads
Download

newer

older

[Einstein Toolkit] #448:...

[Einstein Toolkit] #63: create...

Einstein Toolkit

3 Aug 2011 3 Aug '11

4:38 p.m.

#484: CCTK_CHECK_C_INLINE can define away the inline keyword --------------------+------------------------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Cactus | Version: Keywords: | --------------------+------------------------------------------------------- CCTK_CHECK_C_INLINE tests for an inline keyword that satifies the old pre C99 inline semantics of gcc (gcc pre 4.3 and in particular pre 4.6). If it does not find an inline that behaves like that (which happens eg. with the pgi compiler which does not try to emulate gcc) then configure actually puts a

#define inline

into cctk_Config.h. This removes the keyword from the language turning eg. things like "static inline" into just "static" and the only inlining that happens will be due to the optimizer deciding so.

The reason given for this in aclocal.m4 is the standard conforming behaviour is "incompatible with many libraries, leading to linker errors about duplicate symbols." This does no longer seem to be the case, at least I can compile the ET thornlist fine without this redefinition.

I fix might be to do something like (in aclocal.m4 instead of #defining inline): AC_DEFINE(CCTK_GNU_C_INLINE, ) and AC_DEFINE_UNQUOTED(CCTK_GNU_C_INLINE, $cctk_cv_c_inline) the way that CCTK_CHECK_C_STATIC_INLINE does, instead of redefining a reserved language keyword. This would also affect the "#ifndef inline" part in cctk.h line 180, which currently seems to behave strangely in any case unless the keyword detected by configure is '''exactly''' {{{inline}}} and not e.g. {{{__inline__}}} or {{{__inline}}} or {{{__inline__ __attribute__((__gnu_inline__))}}} that configure also tests for.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/484 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Show replies by date

Einstein Toolkit

3 Aug 3 Aug

5:30 p.m.

#484: CCTK_CHECK_C_INLINE can define away the inline keyword ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Cactus | Version: Resolution: | Keywords: ---------------------+------------------------------------------------------

Comment (by eschnett):

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.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/484#comment:1 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

4 Aug 4 Aug

10:31 a.m.

Comment (by rhaas):

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).

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/484#comment:2 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

12:03 p.m.

Comment (by eschnett):

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.

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

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/484#comment:3 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

4:17 p.m.

Comment (by rhaas):

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.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/484#comment:4 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

13 Aug 13 Aug

10:58 p.m.

Comment (by eschnett):

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.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/484#comment:5 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

14 Aug 14 Aug

11:16 a.m.

Comment (by rhaas):

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.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/484#comment:6 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

12:34 p.m.

Comment (by eschnett):

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.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/484#comment:7 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

1:31 p.m.

Comment (by rhaas):

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.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/484#comment:8 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

3:21 p.m.

Comment (by eschnett):

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.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/484#comment:9 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

3:40 p.m.

Comment (by rhaas):

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.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/484#comment:10 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

17 Aug 17 Aug

8:27 a.m.

#484: CCTK_CHECK_C_INLINE can define away the inline keyword ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: closed Priority: minor | Milestone: Component: Cactus | Version: Resolution: fixed | Keywords: ---------------------+------------------------------------------------------ Changes (by rhaas):

* status: new => closed * resolution: => fixed

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/484#comment:11 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

5532

Age (days ago)

5546

Last active (days ago)

trac@lists.einsteintoolkit.org

11 comments

1 participants

tags (0)

participants (1)

Einstein Toolkit