xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xen: convert lto to Kconfig option
@ 2016-12-05 14:39 Wei Liu
  2016-12-05 15:05 ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2016-12-05 14:39 UTC (permalink / raw)
  To: Xen-devel
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan, Jan Beulich

Introduce CONFIG_LTO in Kconfig. Since this is the last option to be
converted to Kconfig, delete the preceding comment in Rules.mk as well.

Make it depend on BROKEN because it doesn't work at the moment.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>

The option is placed under top level Kconfig because it doesn't fit into
any existing category.

Suggestions are welcome.
---
 xen/Kconfig           |  8 ++++++++
 xen/Rules.mk          | 10 ++--------
 xen/arch/arm/Makefile |  2 +-
 xen/arch/x86/Makefile |  2 +-
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/xen/Kconfig b/xen/Kconfig
index 5515fe9..65d491d 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -30,4 +30,12 @@ config EXPERT
 	string
 	option env="XEN_CONFIG_EXPERT"
 
+config LTO
+	bool "Link Time Optimisation"
+	depends on BROKEN
+	---help---
+	  Enable Link Time Optimisation.
+
+	  If unsure, say N.
+
 source "Kconfig.debug"
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 08cc776..24d13dc 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -1,10 +1,4 @@
 
-#
-# If you change any of these configuration options then you must
-# 'make clean' before rebuilding.
-#
-lto           ?= n
-
 -include $(BASEDIR)/include/config/auto.conf
 
 include $(XEN_ROOT)/Config.mk
@@ -125,7 +119,7 @@ ifeq ($(CONFIG_GCOV),y)
 $(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage
 endif
 
-ifeq ($(lto),y)
+ifeq ($(CONFIG_LTO),y)
 # Would like to handle all object files as bitcode, but objects made from
 # pure asm are in a different format and have to be collected separately.
 # Mirror the directory tree, collecting them as built_in_bin.o.
@@ -144,7 +138,7 @@ built_in.o: $(obj-y)
 ifeq ($(obj-y),)
 	$(CC) $(CFLAGS) -c -x c /dev/null -o $@
 else
-ifeq ($(lto),y)
+ifeq ($(CONFIG_LTO),y)
 	$(LD_LTO) -r -o $@ $^
 else
 	$(LD) $(LDFLAGS) -r -o $@ $^
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index da39d39..f165178 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -81,7 +81,7 @@ $(TARGET).axf: $(TARGET)-syms
 	$(OBJCOPY) --change-addresses +0x80000000 $< $@
 	$(STRIP) $@
 
-ifeq ($(lto),y)
+ifeq ($(CONFIG_LTO),y)
 # Gather all LTO objects together
 prelink_lto.o: $(ALL_OBJS)
 	$(LD_LTO) -r -o $@ $^
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 931917d..fdcf977 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -95,7 +95,7 @@ $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
 
 ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS)
 
-ifeq ($(lto),y)
+ifeq ($(CONFIG_LTO),y)
 # Gather all LTO objects together
 prelink_lto.o: $(ALL_OBJS)
 	$(LD_LTO) -r -o $@ $^
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen: convert lto to Kconfig option
  2016-12-05 14:39 [PATCH] xen: convert lto to Kconfig option Wei Liu
@ 2016-12-05 15:05 ` Jan Beulich
  2016-12-05 15:22   ` Wei Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2016-12-05 15:05 UTC (permalink / raw)
  To: Wei Liu
  Cc: Stefano Stabellini, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Xen-devel

>>> On 05.12.16 at 15:39, <wei.liu2@citrix.com> wrote:
> Introduce CONFIG_LTO in Kconfig. Since this is the last option to be
> converted to Kconfig, delete the preceding comment in Rules.mk as well.
> 
> Make it depend on BROKEN because it doesn't work at the moment.

Looks fine except for this one aspect: Is that brokenness with gcc,
clang, or both? I wouldn't be surprised if it was broken with gcc (in
fact I would be surprised it has ever worked), but I would have
hoped that the clang variant (which iirc this was introduced for)
would work (also e.g. in 4.8).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen: convert lto to Kconfig option
  2016-12-05 15:05 ` Jan Beulich
@ 2016-12-05 15:22   ` Wei Liu
  2016-12-05 15:27     ` Wei Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2016-12-05 15:22 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan, Xen-devel

On Mon, Dec 05, 2016 at 08:05:35AM -0700, Jan Beulich wrote:
> >>> On 05.12.16 at 15:39, <wei.liu2@citrix.com> wrote:
> > Introduce CONFIG_LTO in Kconfig. Since this is the last option to be
> > converted to Kconfig, delete the preceding comment in Rules.mk as well.
> > 
> > Make it depend on BROKEN because it doesn't work at the moment.
> 
> Looks fine except for this one aspect: Is that brokenness with gcc,
> clang, or both? I wouldn't be surprised if it was broken with gcc (in

It's going to be both.

Its build rune is broken.

I got:

  head.S:212:0: fatal error: opening dependency file /..d: Permission denied

That is, this rule

  CFLAGS-y += -MMD -MF $(@D)/.$(@F).d                                             

is evaluated too early outside of prerequisite list, which results in
empty string.

Also I'm sure gcc can't deal with lto because at one point I tried to
fix lto but gcc  toolchain can't cope with that.

> fact I would be surprised it has ever worked), but I would have
> hoped that the clang variant (which iirc this was introduced for)
> would work (also e.g. in 4.8).
> 

I haven't tried clang, because the brokenness is somewhere else.

Wei.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen: convert lto to Kconfig option
  2016-12-05 15:22   ` Wei Liu
@ 2016-12-05 15:27     ` Wei Liu
  2016-12-05 15:58       ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2016-12-05 15:27 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan, Xen-devel

On Mon, Dec 05, 2016 at 03:22:30PM +0000, Wei Liu wrote:
> On Mon, Dec 05, 2016 at 08:05:35AM -0700, Jan Beulich wrote:
> > >>> On 05.12.16 at 15:39, <wei.liu2@citrix.com> wrote:
> > > Introduce CONFIG_LTO in Kconfig. Since this is the last option to be
> > > converted to Kconfig, delete the preceding comment in Rules.mk as well.
> > > 
> > > Make it depend on BROKEN because it doesn't work at the moment.
> > 
> > Looks fine except for this one aspect: Is that brokenness with gcc,
> > clang, or both? I wouldn't be surprised if it was broken with gcc (in
> 
> It's going to be both.
> 
> Its build rune is broken.
> 
> I got:
> 
>   head.S:212:0: fatal error: opening dependency file /..d: Permission denied
> 
> That is, this rule
> 
>   CFLAGS-y += -MMD -MF $(@D)/.$(@F).d                                             
> 
> is evaluated too early outside of prerequisite list, which results in
> empty string.
> 

Sorry, "evaluated too early outside ..." is non-sense. I will just copy
the manpage for automatic variables from Make:

"In particular, you cannot use them anywhere within the target list of a
rule; they have no value there and will expand to the empty string.
Also, they cannot be accessed directly within the prerequisite list of a
rule. A common mistake is attempting to use $@ within the prerequisites
list; this will not work."

The "empty string" part in my description is accurate though.

> Also I'm sure gcc can't deal with lto because at one point I tried to
> fix lto but gcc  toolchain can't cope with that.
> 
> > fact I would be surprised it has ever worked), but I would have
> > hoped that the clang variant (which iirc this was introduced for)
> > would work (also e.g. in 4.8).
> > 
> 
> I haven't tried clang, because the brokenness is somewhere else.
> 
> Wei.
> 
> > Jan
> > 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen: convert lto to Kconfig option
  2016-12-05 15:27     ` Wei Liu
@ 2016-12-05 15:58       ` Jan Beulich
  2016-12-05 16:05         ` Wei Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2016-12-05 15:58 UTC (permalink / raw)
  To: Wei Liu
  Cc: Stefano Stabellini, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Xen-devel

>>> On 05.12.16 at 16:27, <wei.liu2@citrix.com> wrote:
> On Mon, Dec 05, 2016 at 03:22:30PM +0000, Wei Liu wrote:
>> On Mon, Dec 05, 2016 at 08:05:35AM -0700, Jan Beulich wrote:
>> > >>> On 05.12.16 at 15:39, <wei.liu2@citrix.com> wrote:
>> > > Introduce CONFIG_LTO in Kconfig. Since this is the last option to be
>> > > converted to Kconfig, delete the preceding comment in Rules.mk as well.
>> > > 
>> > > Make it depend on BROKEN because it doesn't work at the moment.
>> > 
>> > Looks fine except for this one aspect: Is that brokenness with gcc,
>> > clang, or both? I wouldn't be surprised if it was broken with gcc (in
>> 
>> It's going to be both.
>> 
>> Its build rune is broken.
>> 
>> I got:
>> 
>>   head.S:212:0: fatal error: opening dependency file /..d: Permission denied
>> 
>> That is, this rule
>> 
>>   CFLAGS-y += -MMD -MF $(@D)/.$(@F).d                                           
>   
>> 
>> is evaluated too early outside of prerequisite list, which results in
>> empty string.
>> 
> 
> Sorry, "evaluated too early outside ..." is non-sense. I will just copy
> the manpage for automatic variables from Make:
> 
> "In particular, you cannot use them anywhere within the target list of a
> rule; they have no value there and will expand to the empty string.
> Also, they cannot be accessed directly within the prerequisite list of a
> rule. A common mistake is attempting to use $@ within the prerequisites
> list; this will not work."
> 
> The "empty string" part in my description is accurate though.

And iirc we've run into the issue (in a less destructive way) before
(even more than once), and do some filtering of CFLAGS in a few
places. Apparently we've missed one (or more) ...

Anyway - feel free to put in the patch with my ack, as it certainly
doesn't really alter the status quo here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen: convert lto to Kconfig option
  2016-12-05 15:58       ` Jan Beulich
@ 2016-12-05 16:05         ` Wei Liu
  0 siblings, 0 replies; 6+ messages in thread
From: Wei Liu @ 2016-12-05 16:05 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan, Xen-devel

On Mon, Dec 05, 2016 at 08:58:20AM -0700, Jan Beulich wrote:
> >>> On 05.12.16 at 16:27, <wei.liu2@citrix.com> wrote:
> > On Mon, Dec 05, 2016 at 03:22:30PM +0000, Wei Liu wrote:
> >> On Mon, Dec 05, 2016 at 08:05:35AM -0700, Jan Beulich wrote:
> >> > >>> On 05.12.16 at 15:39, <wei.liu2@citrix.com> wrote:
> >> > > Introduce CONFIG_LTO in Kconfig. Since this is the last option to be
> >> > > converted to Kconfig, delete the preceding comment in Rules.mk as well.
> >> > > 
> >> > > Make it depend on BROKEN because it doesn't work at the moment.
> >> > 
> >> > Looks fine except for this one aspect: Is that brokenness with gcc,
> >> > clang, or both? I wouldn't be surprised if it was broken with gcc (in
> >> 
> >> It's going to be both.
> >> 
> >> Its build rune is broken.
> >> 
> >> I got:
> >> 
> >>   head.S:212:0: fatal error: opening dependency file /..d: Permission denied
> >> 
> >> That is, this rule
> >> 
> >>   CFLAGS-y += -MMD -MF $(@D)/.$(@F).d                                           
> >   
> >> 
> >> is evaluated too early outside of prerequisite list, which results in
> >> empty string.
> >> 
> > 
> > Sorry, "evaluated too early outside ..." is non-sense. I will just copy
> > the manpage for automatic variables from Make:
> > 
> > "In particular, you cannot use them anywhere within the target list of a
> > rule; they have no value there and will expand to the empty string.
> > Also, they cannot be accessed directly within the prerequisite list of a
> > rule. A common mistake is attempting to use $@ within the prerequisites
> > list; this will not work."
> > 
> > The "empty string" part in my description is accurate though.
> 
> And iirc we've run into the issue (in a less destructive way) before
> (even more than once), and do some filtering of CFLAGS in a few
> places. Apparently we've missed one (or more) ...
> 
> Anyway - feel free to put in the patch with my ack, as it certainly
> doesn't really alter the status quo here.
> 

Thanks!

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-12-05 16:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-05 14:39 [PATCH] xen: convert lto to Kconfig option Wei Liu
2016-12-05 15:05 ` Jan Beulich
2016-12-05 15:22   ` Wei Liu
2016-12-05 15:27     ` Wei Liu
2016-12-05 15:58       ` Jan Beulich
2016-12-05 16:05         ` Wei Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).