Skip to content

samtools: Enable -O0 compilation with C99 #6

Merged
merged 1 commit into from
Oct 9, 2017
Merged

samtools: Enable -O0 compilation with C99 #6

merged 1 commit into from
Oct 9, 2017

Conversation

donald
Copy link
Member

@donald donald commented Oct 8, 2017

Compiling samtools with -O0 with gcc might generate an error because of
undefined functions. With C99 an inline function which is neither
declared static nor extern generates no callable function body. However
the compiler is free to use the inline variant or the function body
variant, which is undefined. -O0 forbids inline, so we get a link error.

Add static keyword to inline functions, so that we can compile and link
with -O0 for debugging.

@pmenzel
Copy link
Collaborator

pmenzel commented Oct 9, 2017

Some typos:

  1. s/declated/declared/g
  2. s/if free/is free/

@pmenzel
Copy link
Collaborator

pmenzel commented Oct 9, 2017

Jakub Jelinek commented in GCC bug #81734:

The C99/C11 inlining semantics in this case is that if it is possible and
desirable to inline the function, then it is inlined, but the out-of-line
copy of the inlined function has to be emitted in some other TU.

(In reply to Matthias Klose from comment #2)

but shouldn't these the same with different optimization levels?

Of course not. Whether it is desirable to inline something is an
optimization decision, at -O0 we don't inline anything (unless it has
always_inline attribute), at -Os we inline if the heuristics determines that
inlining will likely make the code smaller, at -O2 etc. if it determines that
inlining will likely make the code faster.

So, for the broken pinfo package, you need to decide what you want. Either
you want it to be inlined always, no matter what, then it needs always_inline
attribute, or if it is not desirable to inline, you want a static copy of the
out of line function in every TU that needs it, then it should be static
inline, or you want out of line copy in one TU and not the others, then you
use what you have above in all but one TU, and in that last TU add extern
void builddircommand(void);, or you want the GNU inline semantics, then you
add gnu_inline attribute and again read the rules for it. See
https://gcc.gnu.org/onlinedocs/gcc-7.1.0/gcc/Inline.html and the C standard
for more details.

Compiling samtools with -O0 with gcc might generate an error because of
undefined functions. With C99 an inline function which is neither
declared static nor extern generates no callable function body. However
the compiler is free to use the inline variant or the function body
variant, which is undefined. -O0 forbids inline, so we get a link error.

Add static keyword to inline functions, so that we can compile and link
with -O0 for debugging.
@donald
Copy link
Member Author

donald commented Oct 9, 2017

Typos fixed, thanks.

I'd put it this way: -O3 worked just by chance, because the compiler happened to decide to use the inline version and not the undefined external version of the function. So it is a bug even if -O3 works most of the time.

@pmenzel pmenzel merged commit 866fa5b into master Oct 9, 2017
Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants