Tim Deegan
2011-Mar-07 11:26 UTC
[Xen-devel] [PATCH 0 of 8] Allow building xen with clang/llvm
This series of patches allows Xen builds with clang/llvm as well as GCC. It only fixes Xen, not tools &c, and only 64-bit, so far. Building Xen with clang isn''t useful in itself (and indeed produces a hypervisor that can be up to 10% slower for a default -O2 build); rather it''s a stepping-stone to doing full link-time optimizations on the LLVM bitcode of the whole hypervisor. I will post a second series to enable that, but it''s not so clean as this one. I think all the changes in this series are reasonable and won''t inconvenience any normal users, but obviously they''re meant for the xen-unstable tree only. Cheers, Tim. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-07 11:26 UTC
[Xen-devel] [PATCH 1 of 8] x86: make spinlock''s 16-bit asm operand explicitly 16-bit
This is needed to compile xen with clang. Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-07 11:26 UTC
[Xen-devel] [PATCH 2 of 8] x86: add explicit size suffixes to some assembly instructions
This is needed to compile xen with clang. Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-07 11:26 UTC
[Xen-devel] [PATCH 3 of 8] x86: redefine a few empty macros as explicit nops
This is needed to compile xen with clang. Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-07 11:26 UTC
[Xen-devel] [PATCH 4 of 8] xen: adjust cpumask initializers to suit clang''s incomplete gccisms
This is needed to compile xen with clang. Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-07 11:26 UTC
[Xen-devel] [PATCH 5 of 8] credit2: remove two nested functions, replacing them with static ones
and passing the outer scope''s variables in explicitly. This is needed to compile xen with clang. Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-07 11:26 UTC
[Xen-devel] [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller
(dump_execution_state()) as its own bug-trap. This is needed to compile xen with clang, which can''t handle using a function name in an asm immediate. Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-07 11:26 UTC
[Xen-devel] [PATCH 7 of 8] x86: redefine REX64_PREFIX for clang, which doesn''t like ''rex64/''
Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-07 11:26 UTC
[Xen-devel] [PATCH 8 of 8] xen: add "clang=y" option to build Xen with clang/llvm instead of gcc
Tested with svn snapshot of clang and llvm from 17 February 2011. Only x86_64 hypervisor builds (make dist-xen clang=y) are supported and I haven''t even begun to look at cross-compiling. Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Keir Fraser
2011-Mar-07 12:03 UTC
Re: [Xen-devel] [PATCH 0 of 8] Allow building xen with clang/llvm
These and the first LTO one are all fine by me. Acked-by: Keir Fraser <keir@xen.org> On 07/03/2011 11:26, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:> This series of patches allows Xen builds with clang/llvm as well as > GCC. It only fixes Xen, not tools &c, and only 64-bit, so far. > > Building Xen with clang isn''t useful in itself (and indeed produces > a hypervisor that can be up to 10% slower for a default -O2 build); > rather it''s a stepping-stone to doing full link-time optimizations > on the LLVM bitcode of the whole hypervisor. I will post a second > series to enable that, but it''s not so clean as this one. > > I think all the changes in this series are reasonable and won''t > inconvenience any normal users, but obviously they''re meant for > the xen-unstable tree only. > > Cheers, > > Tim. > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Ian Campbell
2011-Mar-07 14:54 UTC
Re: [Xen-devel] [PATCH 8 of 8] xen: add "clang=y" option to build Xen with clang/llvm instead of gcc
On Mon, 2011-03-07 at 11:26 +0000, Tim Deegan wrote:> # HG changeset patch > # User Tim Deegan <Tim.Deegan@citrix.com> > # Date 1299496871 0 > # Node ID d91e6a8d36ad3dbf89f42264334baec0cb37063f > # Parent 804007170cf03ea832022e2d589507a3bc0505dc > xen: add "clang=y" option to build Xen with clang/llvm instead of gcc. > > Tested with svn snapshot of clang and llvm from 17 February 2011. > Only x86_64 hypervisor builds (make dist-xen clang=y) are supported > and I haven''t even begun to look at cross-compiling. > > Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> > > diff -r 804007170cf0 -r d91e6a8d36ad Config.mk > --- a/Config.mk Mon Mar 07 11:21:11 2011 +0000 > +++ b/Config.mk Mon Mar 07 11:21:11 2011 +0000 > @@ -148,6 +148,13 @@ CFLAGS += -Wall -Wstrict-prototypes > # result of any casted expression causes a warning. > CFLAGS += -Wno-unused-value > > +ifeq ($(clang),y) > +# Clang complains about macros that expand to ''if ( ( foo == bar ) > ) ...'' > +CFLAGS += -Wno-parentheses > +# And is over-zealous with the printf format lint > +CFLAGS += -Wno-format > +endifIs it worth arranging for "gcc := y" when clang is not enabled? Then a whole bunch of this sort of thing devolves into the CFLAGS-$(a-particular-cc) += -Wfoo pattern.> + > $(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement) > $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement) > > diff -r 804007170cf0 -r d91e6a8d36ad config/StdGNU.mk > --- a/config/StdGNU.mk Mon Mar 07 11:21:11 2011 +0000 > +++ b/config/StdGNU.mk Mon Mar 07 11:21:11 2011 +0000 > @@ -1,6 +1,11 @@ > AS = $(CROSS_COMPILE)as > +ifeq ($(clang),y) > +LD = $(CROSS_COMPILE)gold > +CC = $(CROSS_COMPILE)clang > +else > LD = $(CROSS_COMPILE)ld > CC = $(CROSS_COMPILE)gcc > +endif > CPP = $(CC) -E > AR = $(CROSS_COMPILE)ar > RANLIB = $(CROSS_COMPILE)ranlibLD-$(clang) = ... LD-$(gcc) = ... LD := $(LD-y) ?> @@ -69,5 +74,8 @@ ifneq ($(debug),y) > CFLAGS += -O2 -fomit-frame-pointer > else > # Less than -O1 produces bad code and large stack frames > -CFLAGS += -O1 -fno-omit-frame-pointer -fno-optimize-sibling-calls > +CFLAGS += -O1 -fno-omit-frame-pointer > +ifneq ($(clang),y) > +CFLAGS += -fno-optimize-sibling-callsCFLAGS-$(gcc) += ... etc etc Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Keir Fraser
2011-Mar-07 15:05 UTC
Re: [Xen-devel] [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller
On 07/03/2011 11:26, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:> (dump_execution_state()) as its own bug-trap. > > This is needed to compile xen with clang, which can''t handle using a > function name in an asm immediate.Actually run_in_exception_handler() does have another user, in ns16550.c. Although non-essential, it makes the ''d'' debug key much more useful when running the UART in polled mode. So I suggest we keep run_in_exception_handler but modify it to pass the function pointer in (say) rAX. I think we won''t easily be able to use BUG_STR() logic but r_i_e_h is only used (directly or indirectly) in a few places so the BUG_STR optimisation is unimportant. The only other disadvantage is that rAX is less interesting in the state dump, but any value the function pointer displaces can still be found in the stack dump, albeit with likely a little extra effort. Sound good? -- Keir> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Keir Fraser
2011-Mar-07 15:15 UTC
Re: [Xen-devel] [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller
On 07/03/2011 15:05, "Keir Fraser" <keir.xen@gmail.com> wrote:> On 07/03/2011 11:26, "Tim Deegan" <Tim.Deegan@citrix.com> wrote: > >> (dump_execution_state()) as its own bug-trap. >> >> This is needed to compile xen with clang, which can''t handle using a >> function name in an asm immediate. > > Actually run_in_exception_handler() does have another user, in ns16550.c. > Although non-essential, it makes the ''d'' debug key much more useful when > running the UART in polled mode. > > So I suggest we keep run_in_exception_handler but modify it to pass the > function pointer in (say) rAX.Like the attached patch (against latest tip). -- Keir> I think we won''t easily be able to use > BUG_STR() logic but r_i_e_h is only used (directly or indirectly) in a few > places so the BUG_STR optimisation is unimportant. The only other > disadvantage is that rAX is less interesting in the state dump, but any > value the function pointer displaces can still be found in the stack dump, > albeit with likely a little extra effort. > > Sound good? > > -- Keir > >> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> lists.xensource.com/xen-devel > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
George Dunlap
2011-Mar-07 15:22 UTC
Re: [Xen-devel] [PATCH 5 of 8] credit2: remove two nested functions, replacing them with static ones
Yeah, I kind of knew those nested functions were skanky when I wrote them. :-) Acked-by: George Dunlap <george.dunlap@eu.citrix.com> On Mon, Mar 7, 2011 at 11:26 AM, Tim Deegan <Tim.Deegan@citrix.com> wrote:> and passing the outer scope''s variables in explicitly. > This is needed to compile xen with clang. > > Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > lists.xensource.com/xen-devel > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-07 15:38 UTC
Re: [Xen-devel] [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller
At 15:15 +0000 on 07 Mar (1299510944), Keir Fraser wrote:> On 07/03/2011 15:05, "Keir Fraser" <keir.xen@gmail.com> wrote: > > > On 07/03/2011 11:26, "Tim Deegan" <Tim.Deegan@citrix.com> wrote: > > > >> (dump_execution_state()) as its own bug-trap. > >> > >> This is needed to compile xen with clang, which can''t handle using a > >> function name in an asm immediate. > > > > Actually run_in_exception_handler() does have another user, in ns16550.c. > > Although non-essential, it makes the ''d'' debug key much more useful when > > running the UART in polled mode. > > > > So I suggest we keep run_in_exception_handler but modify it to pass the > > function pointer in (say) rAX. > > Like the attached patch (against latest tip).Sorry, I had missed that other user. I''ll see if I can find a way to make clang use the function address directly; if not, I''d be inclined to have dump_execution_state have its own ID anyway, to keep %rax valid(er) in BUG()s. Tim.> > I think we won''t easily be able to use > > BUG_STR() logic but r_i_e_h is only used (directly or indirectly) in a few > > places so the BUG_STR optimisation is unimportant. The only other > > disadvantage is that rAX is less interesting in the state dump, but any > > value the function pointer displaces can still be found in the stack dump, > > albeit with likely a little extra effort. > > > > Sound good? > > > > -- Keir > > > >> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> > >> > >> > >> _______________________________________________ > >> Xen-devel mailing list > >> Xen-devel@lists.xensource.com > >> lists.xensource.com/xen-devel > > > > >-- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd. (Company #02937203, SL9 0BG) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Keir Fraser
2011-Mar-07 15:44 UTC
Re: [Xen-devel] [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller
On 07/03/2011 15:38, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:> At 15:15 +0000 on 07 Mar (1299510944), Keir Fraser wrote: >> On 07/03/2011 15:05, "Keir Fraser" <keir.xen@gmail.com> wrote: >> >>> On 07/03/2011 11:26, "Tim Deegan" <Tim.Deegan@citrix.com> wrote: >>> >>>> (dump_execution_state()) as its own bug-trap. >>>> >>>> This is needed to compile xen with clang, which can''t handle using a >>>> function name in an asm immediate. >>> >>> Actually run_in_exception_handler() does have another user, in ns16550.c. >>> Although non-essential, it makes the ''d'' debug key much more useful when >>> running the UART in polled mode. >>> >>> So I suggest we keep run_in_exception_handler but modify it to pass the >>> function pointer in (say) rAX. >> >> Like the attached patch (against latest tip). > > Sorry, I had missed that other user. I''ll see if I can find a way to > make clang use the function address directly; if not, I''d be inclined to > have dump_execution_state have its own ID anyway, to keep %rax valid(er) > in BUG()s.But actually dump_execution_state() isn''t used for BUGs and WARNs and ASSERTs. In fact it''s practically dead on x86 -- it''s used in one rather unlikely ACPI error function, and also in __bug/__warn which are never used on x86. And that''s it. Actually the user of run_in_exception_handler() in ns16550.c is really the only one we care about! -- Keir> Tim. > >>> I think we won''t easily be able to use >>> BUG_STR() logic but r_i_e_h is only used (directly or indirectly) in a few >>> places so the BUG_STR optimisation is unimportant. The only other >>> disadvantage is that rAX is less interesting in the state dump, but any >>> value the function pointer displaces can still be found in the stack dump, >>> albeit with likely a little extra effort. >>> >>> Sound good? >>> >>> -- Keir >>> >>>> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> >>>> >>>> >>>> _______________________________________________ >>>> Xen-devel mailing list >>>> Xen-devel@lists.xensource.com >>>> lists.xensource.com/xen-devel >>> >>> >> > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Keir Fraser
2011-Mar-07 15:49 UTC
Re: [Xen-devel] [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller
On 07/03/2011 15:44, "Keir Fraser" <keir.xen@gmail.com> wrote:>> >> Sorry, I had missed that other user. I''ll see if I can find a way to >> make clang use the function address directly; if not, I''d be inclined to >> have dump_execution_state have its own ID anyway, to keep %rax valid(er) >> in BUG()s. > > But actually dump_execution_state() isn''t used for BUGs and WARNs and > ASSERTs. In fact it''s practically dead on x86 -- it''s used in one rather > unlikely ACPI error function, and also in __bug/__warn which are never used > on x86. And that''s it. Actually the user of run_in_exception_handler() in > ns16550.c is really the only one we care about!And furthermore, the GPRs dumped from the ''d'' key via a UART in polled mode are always uninteresting. Because the backtrace will start from ns16550_poll and out from there through Xen''s timer subsystem. The fact we clobber rAX in r_i_e_h() would not make things worse. :-) So if you can''t find a way to force a function address through clang then my patch would be a perfectly fine alternative. -- Keir> -- Keir > >> Tim. >> >>>> I think we won''t easily be able to use >>>> BUG_STR() logic but r_i_e_h is only used (directly or indirectly) in a few >>>> places so the BUG_STR optimisation is unimportant. The only other >>>> disadvantage is that rAX is less interesting in the state dump, but any >>>> value the function pointer displaces can still be found in the stack dump, >>>> albeit with likely a little extra effort. >>>> >>>> Sound good? >>>> >>>> -- Keir >>>> >>>>> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com> >>>>> >>>>> >>>>> _______________________________________________ >>>>> Xen-devel mailing list >>>>> Xen-devel@lists.xensource.com >>>>> lists.xensource.com/xen-devel >>>> >>>> >>> >> >> > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-07 15:56 UTC
Re: [Xen-devel] [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller
At 15:38 +0000 on 07 Mar (1299512321), Tim Deegan wrote:> At 15:15 +0000 on 07 Mar (1299510944), Keir Fraser wrote: > > On 07/03/2011 15:05, "Keir Fraser" <keir.xen@gmail.com> wrote: > > > On 07/03/2011 11:26, "Tim Deegan" <Tim.Deegan@citrix.com> wrote: > > >> This is needed to compile xen with clang, which can''t handle using a > > >> function name in an asm immediate. > > > > > > Actually run_in_exception_handler() does have another user, in ns16550.c. > > > Although non-essential, it makes the ''d'' debug key much more useful when > > > running the UART in polled mode. > > > > > > So I suggest we keep run_in_exception_handler but modify it to pass the > > > function pointer in (say) rAX. > > > > Like the attached patch (against latest tip). > > Sorry, I had missed that other user. I''ll see if I can find a way to > make clang use the function address directly; if not, I''d be inclined to > have dump_execution_state have its own ID anyway, to keep %rax valid(er) > in BUG()s.Turns out to be very straightforward: another level of indirection makes the parser happy. If it''s OK with you, I''ll revert 22987:3147f2d1c6fb and apply this instead: diff -r 3147f2d1c6fb xen/include/asm-x86/bug.h --- a/xen/include/asm-x86/bug.h Mon Mar 07 15:47:59 2011 +0000 +++ b/xen/include/asm-x86/bug.h Mon Mar 07 15:48:32 2011 +0000 @@ -22,7 +22,7 @@ struct bug_frame { asm volatile ( \ "ud2 ; ret %0" BUG_STR(1) \ : : "i" (BUGFRAME_run_fn), \ - "i" (fn) ) + "i" (&(fn)) ) #define WARN() \ asm volatile ( \ Cheers, Tim. -- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd. (Company #02937203, SL9 0BG) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Keir Fraser
2011-Mar-07 16:00 UTC
Re: [Xen-devel] [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller
On 07/03/2011 15:56, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:> At 15:38 +0000 on 07 Mar (1299512321), Tim Deegan wrote: >> At 15:15 +0000 on 07 Mar (1299510944), Keir Fraser wrote: >>> On 07/03/2011 15:05, "Keir Fraser" <keir.xen@gmail.com> wrote: >>> >>> Like the attached patch (against latest tip). >> >> Sorry, I had missed that other user. I''ll see if I can find a way to >> make clang use the function address directly; if not, I''d be inclined to >> have dump_execution_state have its own ID anyway, to keep %rax valid(er) >> in BUG()s. > > Turns out to be very straightforward: another level of indirection makes > the parser happy. If it''s OK with you, I''ll revert 22987:3147f2d1c6fb > and apply this instead:If you''ve successfully both build- and run-tested it with gcc then it''s fine with me. It needs testing as it''s a moderately skanky construct in the first place. -- Keir> diff -r 3147f2d1c6fb xen/include/asm-x86/bug.h > --- a/xen/include/asm-x86/bug.h Mon Mar 07 15:47:59 2011 +0000 > +++ b/xen/include/asm-x86/bug.h Mon Mar 07 15:48:32 2011 +0000 > @@ -22,7 +22,7 @@ struct bug_frame { > asm volatile ( \ > "ud2 ; ret %0" BUG_STR(1) \ > : : "i" (BUGFRAME_run_fn), \ > - "i" (fn) ) > + "i" (&(fn)) ) > > #define WARN() \ > asm volatile ( \ > > Cheers, > > Tim._______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-07 16:06 UTC
Re: [Xen-devel] [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller
At 16:00 +0000 on 07 Mar (1299513606), Keir Fraser wrote:> On 07/03/2011 15:56, "Tim Deegan" <Tim.Deegan@citrix.com> wrote: > > > At 15:38 +0000 on 07 Mar (1299512321), Tim Deegan wrote: > >> At 15:15 +0000 on 07 Mar (1299510944), Keir Fraser wrote: > >>> On 07/03/2011 15:05, "Keir Fraser" <keir.xen@gmail.com> wrote: > >>> > >>> Like the attached patch (against latest tip). > >> > >> Sorry, I had missed that other user. I''ll see if I can find a way to > >> make clang use the function address directly; if not, I''d be inclined to > >> have dump_execution_state have its own ID anyway, to keep %rax valid(er) > >> in BUG()s. > > > > Turns out to be very straightforward: another level of indirection makes > > the parser happy. If it''s OK with you, I''ll revert 22987:3147f2d1c6fb > > and apply this instead: > > If you''ve successfully both build- and run-tested it with gcc then it''s fine > with me. It needs testing as it''s a moderately skanky construct in the first > place.I have, and at least with 64bit gcc 4.4.5 dump_execution_state() still works fine. :) Tim.> -- Keir > > > diff -r 3147f2d1c6fb xen/include/asm-x86/bug.h > > --- a/xen/include/asm-x86/bug.h Mon Mar 07 15:47:59 2011 +0000 > > +++ b/xen/include/asm-x86/bug.h Mon Mar 07 15:48:32 2011 +0000 > > @@ -22,7 +22,7 @@ struct bug_frame { > > asm volatile ( \ > > "ud2 ; ret %0" BUG_STR(1) \ > > : : "i" (BUGFRAME_run_fn), \ > > - "i" (fn) ) > > + "i" (&(fn)) ) > > > > #define WARN() \ > > asm volatile ( \ > > > > Cheers, > > > > Tim. > >-- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd. (Company #02937203, SL9 0BG) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-07 16:29 UTC
Re: [Xen-devel] [PATCH 8 of 8] xen: add "clang=y" option to build Xen with clang/llvm instead of gcc
At 14:54 +0000 on 07 Mar (1299509669), Ian Campbell wrote:> Is it worth arranging for "gcc := y" when clang is not enabled? Then a > whole bunch of this sort of thing devolves into the > CFLAGS-$(a-particular-cc) += -Wfoo > pattern.Something like the attached? It tidies up four such ifeqs, at the cost of one new one to define $(gcc). (4 files changed, 17 insertions(+), 16 deletions(-))> > @@ -1,6 +1,11 @@ > > AS = $(CROSS_COMPILE)as > > +ifeq ($(clang),y) > > +LD = $(CROSS_COMPILE)gold > > +CC = $(CROSS_COMPILE)clang > > +else > > LD = $(CROSS_COMPILE)ld > > CC = $(CROSS_COMPILE)gcc > > +endif > > CPP = $(CC) -E > > AR = $(CROSS_COMPILE)ar > > RANLIB = $(CROSS_COMPILE)ranlib > > LD-$(clang) = ... > LD-$(gcc) = ... > > LD := $(LD-y)I tried that but it looks about as bad, and actually has more repetition. Tim. -- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd. (Company #02937203, SL9 0BG) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Keir Fraser
2011-Mar-07 17:01 UTC
Re: [Xen-devel] [PATCH 8 of 8] xen: add "clang=y" option to build Xen with clang/llvm instead of gcc
On 07/03/2011 16:29, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:> At 14:54 +0000 on 07 Mar (1299509669), Ian Campbell wrote: >> Is it worth arranging for "gcc := y" when clang is not enabled? Then a >> whole bunch of this sort of thing devolves into the >> CFLAGS-$(a-particular-cc) += -Wfoo >> pattern. > > Something like the attached? It tidies up four such ifeqs, at the cost > of one new one to define $(gcc). > (4 files changed, 17 insertions(+), 16 deletions(-))This looks like a definite improvement in readability, to me. I''d like it to be applied. -- Keir>>> @@ -1,6 +1,11 @@ >>> AS = $(CROSS_COMPILE)as >>> +ifeq ($(clang),y) >>> +LD = $(CROSS_COMPILE)gold >>> +CC = $(CROSS_COMPILE)clang >>> +else >>> LD = $(CROSS_COMPILE)ld >>> CC = $(CROSS_COMPILE)gcc >>> +endif >>> CPP = $(CC) -E >>> AR = $(CROSS_COMPILE)ar >>> RANLIB = $(CROSS_COMPILE)ranlib >> >> LD-$(clang) = ... >> LD-$(gcc) = ... >> >> LD := $(LD-y) > > I tried that but it looks about as bad, and actually has more > repetition. > > Tim._______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Jan Beulich
2011-Mar-08 09:27 UTC
Re: [Xen-devel] [PATCH 2 of 8] x86: add explicit size suffixes to some assembly instructions
>>> On 07.03.11 at 12:26, Tim Deegan <Tim.Deegan@citrix.com> wrote: >--- a/xen/arch/x86/x86_emulate/x86_emulate.c Mon Mar 07 11:21:11 2011 +0000 >+++ b/xen/arch/x86/x86_emulate/x86_emulate.c Mon Mar 07 11:21:11 2011 +0000 >@@ -2678,7 +2678,7 @@ x86_emulate( > emulate_fpu_insn_memsrc("fiaddl", src.val); > break; > case 1: /* fimul m64i */ >- emulate_fpu_insn_memsrc("fimul", src.val); >+ emulate_fpu_insn_memsrc("fimuls", src.val);fimull.> break; > case 2: /* ficom m64i */ > emulate_fpu_insn_memsrc("ficoml", src.val);(Side note: The comments in this section are all wrong - they should say "m32i"; there are no 64-bit integer operations other than loads and stores.) Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Ian Campbell
2011-Mar-08 10:00 UTC
Re: [Xen-devel] [PATCH 8 of 8] xen: add "clang=y" option to build Xen with clang/llvm instead of gcc
On Mon, 2011-03-07 at 16:29 +0000, Tim Deegan wrote:> At 14:54 +0000 on 07 Mar (1299509669), Ian Campbell wrote: > > Is it worth arranging for "gcc := y" when clang is not enabled? Then a > > whole bunch of this sort of thing devolves into the > > CFLAGS-$(a-particular-cc) += -Wfoo > > pattern. > > Something like the attached? It tidies up four such ifeqs, at the cost > of one new one to define $(gcc).I think that one is worth it to clean up all the others -- at least all the nastiness will be contained to 1 place. Imagine a far distant future where we also support e.g. icc or something...> (4 files changed, 17 insertions(+), 16 deletions(-)) > > > > @@ -1,6 +1,11 @@ > > > AS = $(CROSS_COMPILE)as > > > +ifeq ($(clang),y) > > > +LD = $(CROSS_COMPILE)gold > > > +CC = $(CROSS_COMPILE)clang > > > +else > > > LD = $(CROSS_COMPILE)ld > > > CC = $(CROSS_COMPILE)gcc > > > +endif > > > CPP = $(CC) -E > > > AR = $(CROSS_COMPILE)ar > > > RANLIB = $(CROSS_COMPILE)ranlib > > > > LD-$(clang) = ... > > LD-$(gcc) = ... > > > > LD := $(LD-y) > > I tried that but it looks about as bad, and actually has more > repetition.Sure. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-08 10:29 UTC
Re: [Xen-devel] [PATCH 8 of 8] xen: add "clang=y" option to build Xen with clang/llvm instead of gcc
At 17:01 +0000 on 07 Mar (1299517318), Keir Fraser wrote:> On 07/03/2011 16:29, "Tim Deegan" <Tim.Deegan@citrix.com> wrote: > > > At 14:54 +0000 on 07 Mar (1299509669), Ian Campbell wrote: > >> Is it worth arranging for "gcc := y" when clang is not enabled? Then a > >> whole bunch of this sort of thing devolves into the > >> CFLAGS-$(a-particular-cc) += -Wfoo > >> pattern. > > > > Something like the attached? It tidies up four such ifeqs, at the cost > > of one new one to define $(gcc). > > (4 files changed, 17 insertions(+), 16 deletions(-)) > > This looks like a definite improvement in readability, to me. I''d like it to > be applied.Done (I''m interpreting this as an Acked-by:). Tim. -- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd. (Company #02937203, SL9 0BG) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Tim Deegan
2011-Mar-08 10:44 UTC
Re: [Xen-devel] [PATCH 2 of 8] x86: add explicit size suffixes to some assembly instructions
At 09:27 +0000 on 08 Mar (1299576427), Jan Beulich wrote:> >>> On 07.03.11 at 12:26, Tim Deegan <Tim.Deegan@citrix.com> wrote: > >--- a/xen/arch/x86/x86_emulate/x86_emulate.c Mon Mar 07 11:21:11 2011 +0000 > >+++ b/xen/arch/x86/x86_emulate/x86_emulate.c Mon Mar 07 11:21:11 2011 +0000 > >@@ -2678,7 +2678,7 @@ x86_emulate( > > emulate_fpu_insn_memsrc("fiaddl", src.val); > > break; > > case 1: /* fimul m64i */ > >- emulate_fpu_insn_memsrc("fimul", src.val); > >+ emulate_fpu_insn_memsrc("fimuls", src.val); > > fimull.fimuls is what the previous code generates, but it does look like that was wrong. I''ll fix it, thanks. Tim.> > break; > > case 2: /* ficom m64i */ > > emulate_fpu_insn_memsrc("ficoml", src.val); > > (Side note: The comments in this section are all wrong - they > should say "m32i"; there are no 64-bit integer operations > other than loads and stores.)-- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd. (Company #02937203, SL9 0BG) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Jan Beulich
2011-Mar-08 11:01 UTC
Re: [Xen-devel] [PATCH 2 of 8] x86: add explicit size suffixes to some assembly instructions
>>> On 08.03.11 at 11:44, Tim Deegan <Tim.Deegan@citrix.com> wrote: > At 09:27 +0000 on 08 Mar (1299576427), Jan Beulich wrote: >> >>> On 07.03.11 at 12:26, Tim Deegan <Tim.Deegan@citrix.com> wrote: >> >--- a/xen/arch/x86/x86_emulate/x86_emulate.c Mon Mar 07 11:21:11 2011 +0000 >> >+++ b/xen/arch/x86/x86_emulate/x86_emulate.c Mon Mar 07 11:21:11 2011 +0000 >> >@@ -2678,7 +2678,7 @@ x86_emulate( >> > emulate_fpu_insn_memsrc("fiaddl", src.val); >> > break; >> > case 1: /* fimul m64i */ >> >- emulate_fpu_insn_memsrc("fimul", src.val); >> >+ emulate_fpu_insn_memsrc("fimuls", src.val); >> >> fimull. > > fimuls is what the previous code generates, but it does look like that > was wrong. I''ll fix it, thanks.And perhaps that fix by itself should go into 4.1/4.0 too. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Keir Fraser
2011-Mar-08 16:29 UTC
Re: [Xen-devel] [PATCH 2 of 8] x86: add explicit size suffixes to some assembly instructions
On 08/03/2011 11:01, "Jan Beulich" <JBeulich@novell.com> wrote:>>>> On 08.03.11 at 11:44, Tim Deegan <Tim.Deegan@citrix.com> wrote: >> At 09:27 +0000 on 08 Mar (1299576427), Jan Beulich wrote: >>>>>> On 07.03.11 at 12:26, Tim Deegan <Tim.Deegan@citrix.com> wrote: >>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c Mon Mar 07 11:21:11 2011 +0000 >>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c Mon Mar 07 11:21:11 2011 +0000 >>>> @@ -2678,7 +2678,7 @@ x86_emulate( >>>> emulate_fpu_insn_memsrc("fiaddl", src.val); >>>> break; >>>> case 1: /* fimul m64i */ >>>> - emulate_fpu_insn_memsrc("fimul", src.val); >>>> + emulate_fpu_insn_memsrc("fimuls", src.val); >>> >>> fimull. >> >> fimuls is what the previous code generates, but it does look like that >> was wrong. I''ll fix it, thanks. > > And perhaps that fix by itself should go into 4.1/4.0 too.Done. Also I fixed the comments to read ''m32i'' and also noted and fixed src.bytes=ea.bytes=4 rather than 8. -- Keir> Jan > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel