| Message ID | 1436388067-11805-1-git-send-email-bill.fischofer@linaro.org |
|---|---|
| State | New |
| Headers | show |
I'm not sure it's sufficient enough. What happens if you cast the type
into something which is not 8 byte aligned, but bigger? The example Mike
saw was in ODP-DPDK CI:
18:50:48 ./include/odp_pool_internal.h:92:9: error: cast from
'odp_buffer_t' (aka 'struct (anonymous at
./include/odp/plat/buffer_types.h:29:9) *') to 'odp_buffer_hdr_t *' (aka
'struct odp_buffer_hdr_t *') increases required alignment from 1 to 64
[-Werror,-Wcast-align]
18:50:48 return (odp_buffer_hdr_t *)buf;
It wouldn't pass with this either, because odp_buffer_hdr_t is 64 byte
aligned. You can't really find an universal size, because that could
force unnecessary alignment restrictions to other types using this macro.
I would still recommend what I wrote in a separate mail chain:
typedef ODP_HANDLE_T(odp_buffer_t, sizeof(odp_buffer_hdr_t));
...
#define odp_handle_t(align) struct { uint8_t unused_dummy_var[align]; } *
/** C/C++ helper macro for strong typing */
#define ODP_HANDLE_T(type, align) odp_handle_t(align) type
That would make this flexible: you can use just 1 if your opaque type is
never casted to a pointer for a different type, or
sizeof(that_different_type) if it is.
The only problem I see in case of the above example, that you don't see
the odp_buffer_hdr_t definition in buffer_types.h
Zoli
On 08/07/15 21:41, Bill Fischofer wrote:
> ODP handles are abstract types however because compilers see their
> implementation they assume these have alignments that can result in false
> lint warnings. Increasing the apparent alignment to uint64_t removes these.
> It has no effect on ODP operation or performance.
>
> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
> ---
> platform/linux-generic/include/odp/plat/strong_types.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp/plat/strong_types.h b/platform/linux-generic/include/odp/plat/strong_types.h
> index a53d763..3eb4556 100644
> --- a/platform/linux-generic/include/odp/plat/strong_types.h
> +++ b/platform/linux-generic/include/odp/plat/strong_types.h
> @@ -17,9 +17,9 @@
>
> /** Use strong typing for ODP types */
> #ifdef __cplusplus
> -#define ODP_HANDLE_T(type) struct _##type { uint8_t unused_dummy_var; } *type
> +#define ODP_HANDLE_T(type) struct _##type { uint64_t unused_dummy_var; } *type
> #else
> -#define odp_handle_t struct { uint8_t unused_dummy_var; } *
> +#define odp_handle_t struct { uint64_t unused_dummy_var; } *
> /** C/C++ helper macro for strong typing */
> #define ODP_HANDLE_T(type) odp_handle_t type
> #endif
>
Good point. Yes, the circular dependencies are problematic and there's also the question of casting to different object types which might have differing alignments. The simplest solution is the (void *) intermediate cast in those specific cases. So please ignore this patch. I'll post the other one for odp-dpdk. The issue in the DPDK code itself is one we can't fix ourselves so I'd ignore it for now. On Thu, Jul 9, 2015 at 5:09 AM, Zoltan Kiss <zoltan.kiss@linaro.org> wrote: > I'm not sure it's sufficient enough. What happens if you cast the type > into something which is not 8 byte aligned, but bigger? The example Mike > saw was in ODP-DPDK CI: > > 18:50:48 ./include/odp_pool_internal.h:92:9: error: cast from > 'odp_buffer_t' (aka 'struct (anonymous at > ./include/odp/plat/buffer_types.h:29:9) *') to 'odp_buffer_hdr_t *' (aka > 'struct odp_buffer_hdr_t *') increases required alignment from 1 to 64 > [-Werror,-Wcast-align] > 18:50:48 return (odp_buffer_hdr_t *)buf; > > It wouldn't pass with this either, because odp_buffer_hdr_t is 64 byte > aligned. You can't really find an universal size, because that could force > unnecessary alignment restrictions to other types using this macro. > I would still recommend what I wrote in a separate mail chain: > > typedef ODP_HANDLE_T(odp_buffer_t, sizeof(odp_buffer_hdr_t)); > ... > #define odp_handle_t(align) struct { uint8_t unused_dummy_var[align]; } * > /** C/C++ helper macro for strong typing */ > #define ODP_HANDLE_T(type, align) odp_handle_t(align) type > > That would make this flexible: you can use just 1 if your opaque type is > never casted to a pointer for a different type, or > sizeof(that_different_type) if it is. > The only problem I see in case of the above example, that you don't see > the odp_buffer_hdr_t definition in buffer_types.h > > Zoli > > > > > On 08/07/15 21:41, Bill Fischofer wrote: > >> ODP handles are abstract types however because compilers see their >> implementation they assume these have alignments that can result in false >> lint warnings. Increasing the apparent alignment to uint64_t removes >> these. >> It has no effect on ODP operation or performance. >> >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> >> --- >> platform/linux-generic/include/odp/plat/strong_types.h | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/platform/linux-generic/include/odp/plat/strong_types.h >> b/platform/linux-generic/include/odp/plat/strong_types.h >> index a53d763..3eb4556 100644 >> --- a/platform/linux-generic/include/odp/plat/strong_types.h >> +++ b/platform/linux-generic/include/odp/plat/strong_types.h >> @@ -17,9 +17,9 @@ >> >> /** Use strong typing for ODP types */ >> #ifdef __cplusplus >> -#define ODP_HANDLE_T(type) struct _##type { uint8_t unused_dummy_var; } >> *type >> +#define ODP_HANDLE_T(type) struct _##type { uint64_t unused_dummy_var; } >> *type >> #else >> -#define odp_handle_t struct { uint8_t unused_dummy_var; } * >> +#define odp_handle_t struct { uint64_t unused_dummy_var; } * >> /** C/C++ helper macro for strong typing */ >> #define ODP_HANDLE_T(type) odp_handle_t type >> #endif >> >>
diff --git a/platform/linux-generic/include/odp/plat/strong_types.h b/platform/linux-generic/include/odp/plat/strong_types.h index a53d763..3eb4556 100644 --- a/platform/linux-generic/include/odp/plat/strong_types.h +++ b/platform/linux-generic/include/odp/plat/strong_types.h @@ -17,9 +17,9 @@ /** Use strong typing for ODP types */ #ifdef __cplusplus -#define ODP_HANDLE_T(type) struct _##type { uint8_t unused_dummy_var; } *type +#define ODP_HANDLE_T(type) struct _##type { uint64_t unused_dummy_var; } *type #else -#define odp_handle_t struct { uint8_t unused_dummy_var; } * +#define odp_handle_t struct { uint64_t unused_dummy_var; } * /** C/C++ helper macro for strong typing */ #define ODP_HANDLE_T(type) odp_handle_t type #endif
ODP handles are abstract types however because compilers see their implementation they assume these have alignments that can result in false lint warnings. Increasing the apparent alignment to uint64_t removes these. It has no effect on ODP operation or performance. Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> --- platform/linux-generic/include/odp/plat/strong_types.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)