Anders, Have you tried to move pragmas out of the spec file?
I think this (plat header file) is the correct place for the pragmas since those are part of the build system and build systems are implementation specific. ODP API _spec_ does not dictate any build system or compiler, and thus should not contain anything extra for any single build system. A valid implementation could implement everything even in a single file (odp/include/odp_api.h). -Petri From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT Savolainen, Petri (Nokia - FI/Espoo) Sent: Friday, April 15, 2016 2:02 PM To: EXT Mike Holmes <mike.hol...@linaro.org>; Maxim Uvarov <maxim.uva...@linaro.org> Cc: lng-odp <lng-odp@lists.linaro.org> Subject: Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible Agree, that it gets ugly (for interface specification readability), if GCC pragmas are added into spec header files. Wouldn’t it work the same way if the pragmas are defined where the spec file is included. Namely e.g. here for the barrier: #ifndef ODP_PLAT_BARRIER_H_ #define ODP_PLAT_BARRIER_H_ #ifdef __cplusplus extern "C" { #endif #include <odp/api/std_types.h> #include <odp/api/atomic.h> #include <odp/api/plat/shared_memory_types.h> #include <odp/api/plat/barrier_types.h> if __GNUC__ >= 4 #pragma GCC visibility push(default) #endif #include <odp/api/spec/barrier.h> if __GNUC__ >= 4 #pragma GCC visibility pop #endif #ifdef __cplusplus } #endif #endif -Petri From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT Mike Holmes Sent: Friday, April 15, 2016 1:44 PM To: Maxim Uvarov <maxim.uva...@linaro.org<mailto:maxim.uva...@linaro.org>> Cc: lng-odp <lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org>> Subject: Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible Maxim it needs to be in the spec, it is the spec that defines the ABI, if you dont do this then everyone implementing will have to manually mark hundreds of other apis as internal rather than we just export the correct list. At least that is my understanding at least. On 15 April 2016 at 06:38, Maxim Uvarov <maxim.uva...@linaro.org<mailto:maxim.uva...@linaro.org>> wrote: Petri, please review this patch. For me it's not clear why it touches include/odp/api/spec files at all. Maxim. On 04/13/16 22:06, Bill Fischofer wrote: Ok, thanks. With that: Reviewed-by: Bill Fischofer <bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org> <mailto:bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org>>> On Wed, Apr 13, 2016 at 2:04 PM, Ricardo Salveti <ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org> <mailto:ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org>>> wrote: On Wed, Apr 13, 2016 at 3:51 PM, Bill Fischofer <bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org> <mailto:bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org>>> wrote: > On Wed, Apr 13, 2016 at 1:47 PM, Ricardo Salveti > <ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org> <mailto:ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org>>> wrote: >> >> On Wed, Apr 13, 2016 at 2:47 PM, Bill Fischofer >> <bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org> <mailto:bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org>>> wrote: >> > >> > On Wed, Apr 13, 2016 at 12:30 PM, Anders Roxell >> > <anders.rox...@linaro.org<mailto:anders.rox...@linaro.org> <mailto:anders.rox...@linaro.org<mailto:anders.rox...@linaro.org>>> >> > wrote: >> >> >> >> Internal functions should not be part of symbols that are visible >> >> outside the library. Using -fvisibility=hidden hides all internal >> >> functions from the public ABI. >> >> >> >> Suggested-by: Ricardo Salveti <ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org> <mailto:ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org>>> >> >> Signed-off-by: Anders Roxell <anders.rox...@linaro.org<mailto:anders.rox...@linaro.org> <mailto:anders.rox...@linaro.org<mailto:anders.rox...@linaro.org>>> >> >> --- >> >> include/odp/api/spec/align.h | 8 ++++++++ >> >> include/odp/api/spec/atomic.h | 8 ++++++++ >> >> include/odp/api/spec/barrier.h | 8 ++++++++ >> >> include/odp/api/spec/buffer.h | 8 ++++++++ >> >> include/odp/api/spec/byteorder.h | 8 ++++++++ >> >> include/odp/api/spec/classification.h | 8 ++++++++ >> >> include/odp/api/spec/compiler.h | 8 ++++++++ >> >> include/odp/api/spec/config.h | 8 ++++++++ >> >> include/odp/api/spec/cpu.h | 8 ++++++++ >> >> include/odp/api/spec/cpumask.h | 8 ++++++++ >> >> include/odp/api/spec/crypto.h | 8 ++++++++ >> >> include/odp/api/spec/debug.h | 8 ++++++++ >> >> include/odp/api/spec/errno.h | 8 ++++++++ >> >> include/odp/api/spec/event.h | 8 ++++++++ >> >> include/odp/api/spec/hash.h | 8 ++++++++ >> >> include/odp/api/spec/hints.h | 8 ++++++++ >> >> include/odp/api/spec/init.h | 8 ++++++++ >> >> include/odp/api/spec/packet.h | 8 ++++++++ >> >> include/odp/api/spec/packet_flags.h | 8 ++++++++ >> >> include/odp/api/spec/packet_io.h | 8 ++++++++ >> >> include/odp/api/spec/packet_io_stats.h | 8 ++++++++ >> >> include/odp/api/spec/pool.h | 8 ++++++++ >> >> include/odp/api/spec/queue.h | 8 ++++++++ >> >> include/odp/api/spec/random.h | 8 ++++++++ >> >> include/odp/api/spec/rwlock.h | 8 ++++++++ >> >> include/odp/api/spec/rwlock_recursive.h | 8 ++++++++ >> >> include/odp/api/spec/schedule.h | 8 ++++++++ >> >> include/odp/api/spec/schedule_types.h | 8 ++++++++ >> >> include/odp/api/spec/shared_memory.h | 8 ++++++++ >> >> include/odp/api/spec/spinlock.h | 8 ++++++++ >> >> include/odp/api/spec/spinlock_recursive.h | 8 ++++++++ >> >> include/odp/api/spec/std_clib.h | 8 ++++++++ >> >> include/odp/api/spec/std_types.h | 8 ++++++++ >> >> include/odp/api/spec/sync.h | 8 ++++++++ >> >> include/odp/api/spec/system_info.h | 8 ++++++++ >> >> include/odp/api/spec/thread.h | 8 ++++++++ >> >> include/odp/api/spec/thrmask.h | 8 ++++++++ >> >> include/odp/api/spec/ticketlock.h | 8 ++++++++ >> >> include/odp/api/spec/time.h | 8 ++++++++ >> >> include/odp/api/spec/timer.h | 8 ++++++++ >> >> include/odp/api/spec/traffic_mngr.h | 8 ++++++++ >> >> include/odp/api/spec/version.h | 8 ++++++++ >> >> platform/Makefile.inc | 1 + >> >> platform/linux-generic/m4/configure.m4 | 12 ++++++++++++ >> >> 44 files changed, 349 insertions(+) >> >> >> >> diff --git a/include/odp/api/spec/align.h >> >> b/include/odp/api/spec/align.h >> >> index 677ff12..027b080 100644 >> >> --- a/include/odp/api/spec/align.h >> >> +++ b/include/odp/api/spec/align.h >> >> @@ -18,6 +18,10 @@ >> >> extern "C" { >> >> #endif >> >> >> >> +#if __GNUC__ >= 4 >> >> +#pragma GCC visibility push(default) >> >> +#endif >> > >> > >> > Do these need to be guarded? Do we care about GCC < 4 at this point? >> > How >> > does this affect clang? >> >> It's usually good to protect that with the supported GCC version, but >> then the question is if the ODP project itself would be fine to drop >> support for GCC < 4. >> >> And for clang, it doesn't affect since it also works fine with it. > > > So clang exports symbol __GNUC__ with a value of at least 4? Yes, https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/InitPreprocessor.cpp#L494 Added quite a while ago. Cheers, -- Ricardo Salveti _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> https://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> https://lists.linaro.org/mailman/listinfo/lng-odp -- Mike Holmes Technical Manager - Linaro Networking Group Linaro.org<http://www.linaro.org/> │ Open source software for ARM SoCs "Work should be fun and collaborative, the rest follows"
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
