| Message ID | 1395766541-23979-32-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Deferred, archived |
| Headers | show |
On Tue, 2014-03-25 at 16:55 +0000, Julien Grall wrote: > Clang 3.5 will fail to build most of the tools because aligned attribute > is not used sometimes: > > In file included from xc_core.c:64: > In file included from ./xg_private.h:30: > In file included from ./xenctrl.h:55: > ../../tools/include/xen/foreign/x86_64.h:198:47: error: 'aligned' attribute ignored when parsing type [-Werror,-Wignored-attributes] > __align8__ uint64_t evtchn_pending[sizeof(__align8__ uint64_t) * 8]; > ^~~~~~~~~~ > ../../tools/include/xen/foreign/x86_64.h:13:36: note: expanded from macro '__align8__' > # define __align8__ __attribute__((aligned (8))) > ^~~~~~~~~~~ > ../../tools/include/xen/foreign/x86_64.h:199:44: error: 'aligned' attribute ignored when parsing type [-Werror,-Wignored-attributes] > __align8__ uint64_t evtchn_mask[sizeof(__align8__ uint64_t) * 8]; Is sizeof(__align8__ uint64_t) != sizeof(uint64_t) under any circumstances? IOW can't we just drop the __align8__ here? > ^~~~~~~~~~ > ../../tools/include/xen/foreign/x86_64.h:13:36: note: expanded from macro '__align8__' > # define __align8__ __attribute__((aligned (8))) > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > --- > tools/Rules.mk | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tools/Rules.mk b/tools/Rules.mk > index 13d8fc1..6fb746f 100644 > --- a/tools/Rules.mk > +++ b/tools/Rules.mk > @@ -17,6 +17,8 @@ XEN_LIBXENSTAT = $(XEN_ROOT)/tools/xenstat/libxenstat/src > XEN_BLKTAP2 = $(XEN_ROOT)/tools/blktap2 > XEN_LIBVCHAN = $(XEN_ROOT)/tools/libvchan > > +CFLAGS-$(clang) += -Wno-ignored-attributes > + > CFLAGS_xeninclude = -I$(XEN_INCLUDE) > > CFLAGS_libxenctrl = -I$(XEN_LIBXC) $(CFLAGS_xeninclude)
On 03/27/2014 05:14 PM, Ian Campbell wrote: > On Tue, 2014-03-25 at 16:55 +0000, Julien Grall wrote: >> Clang 3.5 will fail to build most of the tools because aligned attribute >> is not used sometimes: >> >> In file included from xc_core.c:64: >> In file included from ./xg_private.h:30: >> In file included from ./xenctrl.h:55: >> ../../tools/include/xen/foreign/x86_64.h:198:47: error: 'aligned' attribute ignored when parsing type [-Werror,-Wignored-attributes] >> __align8__ uint64_t evtchn_pending[sizeof(__align8__ uint64_t) * 8]; >> ^~~~~~~~~~ >> ../../tools/include/xen/foreign/x86_64.h:13:36: note: expanded from macro '__align8__' >> # define __align8__ __attribute__((aligned (8))) >> ^~~~~~~~~~~ >> ../../tools/include/xen/foreign/x86_64.h:199:44: error: 'aligned' attribute ignored when parsing type [-Werror,-Wignored-attributes] >> __align8__ uint64_t evtchn_mask[sizeof(__align8__ uint64_t) * 8]; > > Is sizeof(__align8__ uint64_t) != sizeof(uint64_t) under any > circumstances? IOW can't we just drop the __align8__ here? I don't think. This code is generated by mkheader.py. I will look at it and see if I can modify it. Regards,
On Sat, 2014-09-13 at 18:42 +0000, Marcin Cieslak wrote:
The commit message could go into a bit more detail as to why this is not
allowed. To me it sounds like clang is being overly picky, surely it can
just ignore the alignment in that context?
> +typesizes["arm32"] = {
Duplicating the entire inttypes list is not really very nice.
I would prefer instead to make the existing inttypes more structured and
separate the type itself and the other modifiers/attributes into fields
(of a class or in a tuple) and put them together in the right way as we
need them. Methods on the class or helpers (type.gen_type() gen_sizeof()
etc) would help keep this clean.
Ian.
Hi Ian, On 16/09/14 09:21, Ian Campbell wrote: > On Sat, 2014-09-13 at 18:42 +0000, Marcin Cieslak wrote: > > The commit message could go into a bit more detail as to why this is not > allowed. To me it sounds like clang is being overly picky, surely it can > just ignore the alignment in that context? Clang will throw a warning when an attribute is unused (see -Wignored-attributes). In case of libxc, we compiled with -Werror which will turn this warning into an error. I sent a patch few months ago [1] to disable this option. One of the solution was to modify mkheader.py. Regards,
On Tue, 2014-09-16 at 11:26 -0700, Julien Grall wrote: > Hi Ian, > > On 16/09/14 09:21, Ian Campbell wrote: > > On Sat, 2014-09-13 at 18:42 +0000, Marcin Cieslak wrote: > > > > The commit message could go into a bit more detail as to why this is not > > allowed. To me it sounds like clang is being overly picky, surely it can > > just ignore the alignment in that context? > > Clang will throw a warning when an attribute is unused (see > -Wignored-attributes). In case of libxc, we compiled with -Werror which > will turn this warning into an error. Sounds to me like -Wignored-attributes is a bit pointless, perhaps we shouldn't use it? Ian.
>>> On 16.09.14 at 20:32, <ian.campbell@citrix.com> wrote: > On Tue, 2014-09-16 at 11:26 -0700, Julien Grall wrote: >> Hi Ian, >> >> On 16/09/14 09:21, Ian Campbell wrote: >> > On Sat, 2014-09-13 at 18:42 +0000, Marcin Cieslak wrote: >> > >> > The commit message could go into a bit more detail as to why this is not >> > allowed. To me it sounds like clang is being overly picky, surely it can >> > just ignore the alignment in that context? >> >> Clang will throw a warning when an attribute is unused (see >> -Wignored-attributes). In case of libxc, we compiled with -Werror which >> will turn this warning into an error. > > Sounds to me like -Wignored-attributes is a bit pointless, perhaps we > shouldn't use it? +1 Jan
Hi, On 17/09/14 03:50, Jan Beulich wrote: >>>> On 16.09.14 at 20:32, <ian.campbell@citrix.com> wrote: >> On Tue, 2014-09-16 at 11:26 -0700, Julien Grall wrote: >>> Hi Ian, >>> >>> On 16/09/14 09:21, Ian Campbell wrote: >>>> On Sat, 2014-09-13 at 18:42 +0000, Marcin Cieslak wrote: >>>> >>>> The commit message could go into a bit more detail as to why this is not >>>> allowed. To me it sounds like clang is being overly picky, surely it can >>>> just ignore the alignment in that context? >>> >>> Clang will throw a warning when an attribute is unused (see >>> -Wignored-attributes). In case of libxc, we compiled with -Werror which >>> will turn this warning into an error. >> >> Sounds to me like -Wignored-attributes is a bit pointless, perhaps we >> shouldn't use it? > > +1 Ian, shall I resend the patch which disabled this attribute? https://patches.linaro.org/27062/ Regards,
diff --git a/tools/Rules.mk b/tools/Rules.mk index 13d8fc1..6fb746f 100644 --- a/tools/Rules.mk +++ b/tools/Rules.mk @@ -17,6 +17,8 @@ XEN_LIBXENSTAT = $(XEN_ROOT)/tools/xenstat/libxenstat/src XEN_BLKTAP2 = $(XEN_ROOT)/tools/blktap2 XEN_LIBVCHAN = $(XEN_ROOT)/tools/libvchan +CFLAGS-$(clang) += -Wno-ignored-attributes + CFLAGS_xeninclude = -I$(XEN_INCLUDE) CFLAGS_libxenctrl = -I$(XEN_LIBXC) $(CFLAGS_xeninclude)
Clang 3.5 will fail to build most of the tools because aligned attribute is not used sometimes: In file included from xc_core.c:64: In file included from ./xg_private.h:30: In file included from ./xenctrl.h:55: ../../tools/include/xen/foreign/x86_64.h:198:47: error: 'aligned' attribute ignored when parsing type [-Werror,-Wignored-attributes] __align8__ uint64_t evtchn_pending[sizeof(__align8__ uint64_t) * 8]; ^~~~~~~~~~ ../../tools/include/xen/foreign/x86_64.h:13:36: note: expanded from macro '__align8__' # define __align8__ __attribute__((aligned (8))) ^~~~~~~~~~~ ../../tools/include/xen/foreign/x86_64.h:199:44: error: 'aligned' attribute ignored when parsing type [-Werror,-Wignored-attributes] __align8__ uint64_t evtchn_mask[sizeof(__align8__ uint64_t) * 8]; ^~~~~~~~~~ ../../tools/include/xen/foreign/x86_64.h:13:36: note: expanded from macro '__align8__' # define __align8__ __attribute__((aligned (8))) Signed-off-by: Julien Grall <julien.grall@linaro.org> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> --- tools/Rules.mk | 2 ++ 1 file changed, 2 insertions(+)