powerpc: crtsaveres.o needed only when -Os flag is enabled

PATCH In review Ram Pai 4 participants 6 messages Apr 29, 2014 – May 2, 2014 PowerPC

lore

Share & tools

lore

Ram Pai

author

2014-04-29 00:06 UTC #0 lore raw

PATCH 1/1 powerpc: crtsaveres.o needed only when -Os flag is enabled

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 <[email protected]>

arch/powerpc/Makefile +3 -0

index 4c0cedf..cf12f38 100644
@@ -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)

Brian W Hart

Tested-by on v1 1/1

2014-04-29 15:38 UTC #1 lore raw

Quoted 18 lines · Ram Pai

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 <[email protected]>
arch/powerpc/Makefile +0 -0
index 4c0cedf..cf12f38 100644

arch/powerpc/Makefile CPP = $(CC) -E $(KBUILD_CFLAGS) · line 166

Quoted 10 lines

arch/powerpc/Makefile +3 -0

@@ -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 <[email protected]>

Benjamin Herrenschmidt

2014-04-30 05:15 UTC #2 lore raw

Quoted 16 lines · Brian W Hart

arch/powerpc/Makefile CPP = $(CC) -E $(KBUILD_CFLAGS) · line 166
Quoted 9 lines
CHECKFLAGS += -m$(CONFIG_WORD_SIZE) -D__powerpc__ -D__powerpc$(CONFIG_WORD_SIZE)__
arch/powerpc/Makefile +3 -0
+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).

Quoted 1 line

Tested-by Brian W Hart <[email protected]>

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.

Ram Pai

author

2014-04-30 18:43 UTC #3 lore raw

Quoted 18 lines · Benjamin Herrenschmidt

Quoted 16 lines · Brian W Hart
arch/powerpc/Makefile CPP = $(CC) -E $(KBUILD_CFLAGS) · line 166
Quoted 9 lines
CHECKFLAGS += -m$(CONFIG_WORD_SIZE) -D__powerpc__ -D__powerpc$(CONFIG_WORD_SIZE)__
arch/powerpc/Makefile +3 -0
+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).

Quoted 7 lines

Quoted 1 line
Tested-by Brian W Hart <[email protected]>
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?

Quoted 4 lines

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.

Quoted 7 lines

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?

Quoted 2 lines

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 :)

Aneesh Kumar K.V

2014-05-02 05:39 UTC #4 lore raw

In reply to Ram Pai

Quoted 19 lines · Ram Pai

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 <[email protected]>
arch/powerpc/Makefile +0 -0
index 4c0cedf..cf12f38 100644

arch/powerpc/Makefile CPP = $(CC) -E $(KBUILD_CFLAGS) · line 166

Quoted 10 lines

arch/powerpc/Makefile +3 -0

@@ -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

Ram Pai

author

2014-05-02 17:47 UTC #5 lore raw

Quoted 28 lines · Aneesh Kumar K.V

Quoted 26 lines · Ram Pai
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 <[email protected]>
arch/powerpc/Makefile +3 -0
index 4c0cedf..cf12f38 100644
@@ -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
+
 

Quoted 6 lines

arch/powerpc/Makefile CPP = $(CC) -E $(KBUILD_CFLAGS) · line 166
Quoted 3 lines
arch/powerpc/Makefile +0 -0
 # 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.