* [PATCH 1/1] powerpc: crtsaveres.o needed only when -Os flag is enabled
@ 2014-04-29 0:05 Ram Pai
2014-04-29 15:38 ` Brian W Hart
2014-05-02 5:39 ` Aneesh Kumar K.V
0 siblings, 2 replies; 6+ messages in thread
From: Ram Pai @ 2014-04-29 0:05 UTC (permalink / raw)
To: benh, paulus; +Cc: Ram Pai, linuxppc-dev, hartb, anton, tony
powerpc: crtsaveres.o needed only when -Os flag is enabled
Currently on powerpc arch, out-of-tree module fails to build without
crtsaveres.o, even when the module has no dependency on the symbols
provided by the file; when built without the -Os flag.
BTW: '-Os' flag is enabled when CONFIG_CC_OPTIMIZE_FOR_SIZE is
configured.
This patch fixes that problem.
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 4c0cedf..cf12f38 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -157,7 +157,10 @@ CPP = $(CC) -E $(KBUILD_CFLAGS)
CHECKFLAGS += -m$(CONFIG_WORD_SIZE) -D__powerpc__ -D__powerpc$(CONFIG_WORD_SIZE)__
+ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
+endif
+
# No AltiVec or VSX instructions when building kernel
KBUILD_CFLAGS += $(call cc-option,-mno-altivec)
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] powerpc: crtsaveres.o needed only when -Os flag is enabled
2014-04-29 0:05 [PATCH 1/1] powerpc: crtsaveres.o needed only when -Os flag is enabled Ram Pai
@ 2014-04-29 15:38 ` Brian W Hart
2014-04-30 5:14 ` Benjamin Herrenschmidt
2014-05-02 5:39 ` Aneesh Kumar K.V
1 sibling, 1 reply; 6+ messages in thread
From: Brian W Hart @ 2014-04-29 15:38 UTC (permalink / raw)
To: linuxppc-dev
On Mon, Apr 28, 2014 at 05:05:08PM -0700, Ram Pai wrote:
> powerpc: crtsaveres.o needed only when -Os flag is enabled
>
> Currently on powerpc arch, out-of-tree module fails to build without
> crtsaveres.o, even when the module has no dependency on the symbols
> provided by the file; when built without the -Os flag.
>
> BTW: '-Os' flag is enabled when CONFIG_CC_OPTIMIZE_FOR_SIZE is
> configured.
>
> This patch fixes that problem.
>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 4c0cedf..cf12f38 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -157,7 +157,10 @@ CPP = $(CC) -E $(KBUILD_CFLAGS)
>
> CHECKFLAGS += -m$(CONFIG_WORD_SIZE) -D__powerpc__ -D__powerpc$(CONFIG_WORD_SIZE)__
>
> +ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
> KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> +endif
> +
>
> # No AltiVec or VSX instructions when building kernel
> KBUILD_CFLAGS += $(call cc-option,-mno-altivec)
I didn't try building a kernel or in-tree modules, but I confirmed
that it allows building of out-of-tree modules when crtsavres.o is
not present (e.g. as for a distro install where the kernel headers
are provided by package, rather than being manually prepared from
the sources).
Tested-by: Brian W Hart <hartb@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] powerpc: crtsaveres.o needed only when -Os flag is enabled
2014-04-29 15:38 ` Brian W Hart
@ 2014-04-30 5:14 ` Benjamin Herrenschmidt
2014-04-30 18:43 ` Ram Pai
0 siblings, 1 reply; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2014-04-30 5:14 UTC (permalink / raw)
To: Ram Pai; +Cc: linuxppc-dev, Brian W Hart
On Tue, 2014-04-29 at 10:38 -0500, Brian W Hart wrote:
> > CHECKFLAGS += -m$(CONFIG_WORD_SIZE) -D__powerpc__ -D__powerpc$(CONFIG_WORD_SIZE)__
> >
> > +ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
> > KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> > +endif
> > +
> >
> > # No AltiVec or VSX instructions when building kernel
> > KBUILD_CFLAGS += $(call cc-option,-mno-altivec)
>
> I didn't try building a kernel or in-tree modules, but I confirmed
> that it allows building of out-of-tree modules when crtsavres.o is
> not present (e.g. as for a distro install where the kernel headers
> are provided by package, rather than being manually prepared from
> the sources).
>
> Tested-by: Brian W Hart <hartb@linux.vnet.ibm.com>
I still don't like it. What guarantee do we have that gcc will never
call into this with other optimisation settings ? It might decide
one day that calling out for saving a large pile of registers
is still more efficient than unrolling the whole lot, including
for speed.
Besides that doesn't fix the root problem. We want to be able to
build the kernel with CONFIG_CC_OPTIMIZE_FOR_SIZE and still have
modules.
So a better solution needs to be found. I don't know what that
solution is (we might want to look at what other archs are doing
maybe ?), could be to include crtsaveres.S in the build of every
module (we really don't want to EXPORT_SYMBOL these guys), but
that would mean having it installed somewhere with the kernel
headers for out-of-tree modules...
If necessary, involve lkml, Rusty etc... but this patch is crap.
Ben.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] powerpc: crtsaveres.o needed only when -Os flag is enabled
2014-04-30 5:14 ` Benjamin Herrenschmidt
@ 2014-04-30 18:43 ` Ram Pai
0 siblings, 0 replies; 6+ messages in thread
From: Ram Pai @ 2014-04-30 18:43 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Brian W Hart
On Wed, Apr 30, 2014 at 03:14:54PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2014-04-29 at 10:38 -0500, Brian W Hart wrote:
>
> > > CHECKFLAGS += -m$(CONFIG_WORD_SIZE) -D__powerpc__ -D__powerpc$(CONFIG_WORD_SIZE)__
> > >
> > > +ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
> > > KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> > > +endif
> > > +
> > >
> > > # No AltiVec or VSX instructions when building kernel
> > > KBUILD_CFLAGS += $(call cc-option,-mno-altivec)
> >
> > I didn't try building a kernel or in-tree modules, but I confirmed
> > that it allows building of out-of-tree modules when crtsavres.o is
> > not present (e.g. as for a distro install where the kernel headers
> > are provided by package, rather than being manually prepared from
> > the sources).
> >
> > Tested-by: Brian W Hart <hartb@linux.vnet.ibm.com>
>
> I still don't like it. What guarantee do we have that gcc will never
> call into this with other optimisation settings ? It might decide
> one day that calling out for saving a large pile of registers
> is still more efficient than unrolling the whole lot, including
> for speed.
This patch operates on the assumption that arch/powerpc/lib/crtsavres.o
is needed only if the code is compiled with -Os. Are you saying
this assumption is wrong?
>
> Besides that doesn't fix the root problem. We want to be able to
> build the kernel with CONFIG_CC_OPTIMIZE_FOR_SIZE and still have
> modules.
And this patch will not stop you from doing that. You can compile your
kernel with CONFIG_CC_OPTIMIZE_FOR_SIZE and modules will be built
because arch/powerpc/lib/crtsavres.o will be linked with the module.
Now, if the arch/powerpc/lib/crtsavres.o file does not exist, that
is a different problem and has to be fixed by the distros for
out-of-tree modules.
>
> So a better solution needs to be found. I don't know what that
> solution is (we might want to look at what other archs are doing
> maybe ?), could be to include crtsaveres.S in the build of every
> module (we really don't want to EXPORT_SYMBOL these guys), but
> that would mean having it installed somewhere with the kernel
> headers for out-of-tree modules...
Currently crtsaveres.o is expected to be in the build during
the linking stage of the module. You suggest instead have
crtsaveres.S and get it compiled and linked?
>
> If necessary, involve lkml, Rusty etc... but this patch is crap.
I dont see other archs having this problem. Possibly because there
linker have inbuilt capabilities to satisfy the missing symbols?
Alan Modra did mention that the ppc linker will soon have the
capability to handle -Os compiled code, without the help
of arch/powerpc/lib/crtsavres.o.
However this patch is not about having crtsavres.o or crtsaveres.S
Its about not needing crtsavres.o if the code is not compiled for
space optimization using -Os. If you say that the assumption
is wrong, than yes the code is crap :)
RP
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] powerpc: crtsaveres.o needed only when -Os flag is enabled
2014-04-29 0:05 [PATCH 1/1] powerpc: crtsaveres.o needed only when -Os flag is enabled Ram Pai
2014-04-29 15:38 ` Brian W Hart
@ 2014-05-02 5:39 ` Aneesh Kumar K.V
2014-05-02 17:47 ` Ram Pai
1 sibling, 1 reply; 6+ messages in thread
From: Aneesh Kumar K.V @ 2014-05-02 5:39 UTC (permalink / raw)
To: Ram Pai, benh, paulus; +Cc: linuxppc-dev, Ram Pai, anton, tony, hartb
Ram Pai <linuxram@us.ibm.com> writes:
> powerpc: crtsaveres.o needed only when -Os flag is enabled
>
> Currently on powerpc arch, out-of-tree module fails to build without
> crtsaveres.o, even when the module has no dependency on the symbols
> provided by the file; when built without the -Os flag.
>
> BTW: '-Os' flag is enabled when CONFIG_CC_OPTIMIZE_FOR_SIZE is
> configured.
>
> This patch fixes that problem.
>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 4c0cedf..cf12f38 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -157,7 +157,10 @@ CPP = $(CC) -E $(KBUILD_CFLAGS)
>
> CHECKFLAGS += -m$(CONFIG_WORD_SIZE) -D__powerpc__ -D__powerpc$(CONFIG_WORD_SIZE)__
>
> +ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
> KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> +endif
> +
>
> # No AltiVec or VSX instructions when building kernel
> KBUILD_CFLAGS += $(call cc-option,-mno-altivec)
>
So if we enable CONFIG_CC_OPTIMIZE_FOR_SIZE can we build out-of-tree
module with this patch ?
-aneesh
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] powerpc: crtsaveres.o needed only when -Os flag is enabled
2014-05-02 5:39 ` Aneesh Kumar K.V
@ 2014-05-02 17:47 ` Ram Pai
0 siblings, 0 replies; 6+ messages in thread
From: Ram Pai @ 2014-05-02 17:47 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: hartb, paulus, anton, tony, linuxppc-dev
On Fri, May 02, 2014 at 11:09:17AM +0530, Aneesh Kumar K.V wrote:
> Ram Pai <linuxram@us.ibm.com> writes:
>
> > powerpc: crtsaveres.o needed only when -Os flag is enabled
> >
> > Currently on powerpc arch, out-of-tree module fails to build without
> > crtsaveres.o, even when the module has no dependency on the symbols
> > provided by the file; when built without the -Os flag.
> >
> > BTW: '-Os' flag is enabled when CONFIG_CC_OPTIMIZE_FOR_SIZE is
> > configured.
> >
> > This patch fixes that problem.
> >
> > Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> >
> > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> > index 4c0cedf..cf12f38 100644
> > --- a/arch/powerpc/Makefile
> > +++ b/arch/powerpc/Makefile
> > @@ -157,7 +157,10 @@ CPP = $(CC) -E $(KBUILD_CFLAGS)
> >
> > CHECKFLAGS += -m$(CONFIG_WORD_SIZE) -D__powerpc__ -D__powerpc$(CONFIG_WORD_SIZE)__
> >
> > +ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
> > KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> > +endif
> > +
> >
> > # No AltiVec or VSX instructions when building kernel
> > KBUILD_CFLAGS += $(call cc-option,-mno-altivec)
> >
>
> So if we enable CONFIG_CC_OPTIMIZE_FOR_SIZE can we build out-of-tree
> module with this patch ?
Yes, provided crtsaveres.o is available.
If crtsaveres.o is not available; some distro dont ship it, than
out-of-tree module linking will fail.
Some distro don't ship crtsaveres.o since they do not want
kernel and modules to be built with space optimization. In such cases
requiring crtsaveres.o to be available during module building, even
when the module is not built for space optimization is wrong.
This patch fixes that issue.
RP
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-05-02 17:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-29 0:05 [PATCH 1/1] powerpc: crtsaveres.o needed only when -Os flag is enabled Ram Pai
2014-04-29 15:38 ` Brian W Hart
2014-04-30 5:14 ` Benjamin Herrenschmidt
2014-04-30 18:43 ` Ram Pai
2014-05-02 5:39 ` Aneesh Kumar K.V
2014-05-02 17:47 ` Ram Pai
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).