> On Jan. 25, 2015, 11:36 p.m., Diederik de Groot wrote: > > /branches/11/include/asterisk/inline_api.h, lines 44-51 > > <https://reviewboard.asterisk.org/r/4370/diff/1/?file=71056#file71056line44> > > > > You might want to check out: > > #define forceinline __inline__ __attribute__((always_inline)) > > #define ensure_forceinline __attribute__((always_inline)) // inline > > or die > > > > To see if they would help forcing clang to inline > > Matt Jordan wrote: > I gave it a shot, and clang still believes that the functions are > re-defined. > > There may be a magical invocation to get clang to believe that these > inlinings should occur, but I'm not sure what it is. > > Diederik de Groot wrote: > I was intriged by the inline issue a little. There must be some solution. > Declaring the function static ought to help against the multiple definition > warning. Be cause the goal is to inline the function, we already expect the > function content to be replicated all over the place, so the argument in the > comment in include/asterisk/inline_api.h line 30 doesn't completely hold up. > > For the non-inlined version, static will not really be problematic, as > the clang compiler (even in -O0 mode) will optimize the multiple declarations > out (as opposed to gcc, which would not do this in -O0 mode, resulting in a > bigger binary then necessary). > > So: > --- include/asterisk/inline_api.h 2015-02-20 14:11:49.011228989 +0100 > +++ include/asterisk/inline_api.h~ 2015-02-20 14:11:38.000000000 +0100 > @@ -52,7 +52,7 @@ > > #if !defined(AST_API_MODULE) > #if defined(__clang__) > -#define AST_INLINE_API(hdr, body) static hdr; static inline hdr body > +#define AST_INLINE_API(hdr, body) static hdr; static forceinline hdr body > #else > #define AST_INLINE_API(hdr, body) hdr; extern inline hdr body > #endif > > Seems to do the trick, as far as i can tell. Can you verify this ? > > Diederik de Groot wrote: > Diff went a little astray: > Final result should have looked like: > #if !defined(AST_API_MODULE) > #if defined(__clang__) > #define AST_INLINE_API(hdr, body) static hdr; static inline hdr body > #else > #define AST_INLINE_API(hdr, body) hdr; extern inline hdr body > #endif > #else > #define AST_INLINE_API(hdr, body) hdr; hdr body > #endif >
Yup, that works just fine. I haven't disassembled anything to see what it actually generated, but it certainly compiled and ran. That meets my litmus test for this patch! - Matt ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4370/#review14280 ----------------------------------------------------------- On Feb. 19, 2015, 10:09 p.m., Matt Jordan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4370/ > ----------------------------------------------------------- > > (Updated Feb. 19, 2015, 10:09 p.m.) > > > Review request for Asterisk Developers and Diederik de Groot. > > > Bugs: ASTERISK-20850 > https://issues.asterisk.org/jira/browse/ASTERISK-20850 > > > Repository: Asterisk > > > Description > ------- > > This is a continuation of the patch put up for review on r3488. It addresses > the issues found on that review. > > This patch *should* make Asterisk compile under clang. Note that compiling > with --enable-dev-mode will cause Asterisk to fail to compile under clang, as > it detects a number of warnings that aren't fixed under this patch. > > > Diffs > ----- > > /branches/11/makeopts.in 432011 > /branches/11/main/Makefile 432011 > /branches/11/include/asterisk/utils.h 432011 > /branches/11/include/asterisk/inline_api.h 432011 > /branches/11/configure.ac 432011 > /branches/11/configure UNKNOWN > /branches/11/Makefile 432011 > > Diff: https://reviewboard.asterisk.org/r/4370/diff/ > > > Testing > ------- > > * Compiled Asterisk with and without --enable-dev-mode using gcc. Asterisk > compiles correctly. > * Compiled Asterisk without --enable-dev-mode using clang. Asterisk compiles, > links, and executes. > > Note that you will need the BlocksRuntime to run Asterisk when it is compiled > with clang. > > > Thanks, > > Matt Jordan > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
