diff mbox

[U-Boot,v3] Switch from archive libraries to partial linking

Message ID 201011191908.00162.vapier@gentoo.org
State Superseded
Headers show

Commit Message

Mike Frysinger Nov. 20, 2010, 12:07 a.m. UTC
On Friday, November 19, 2010 03:03:54 Wolfgang Denk wrote:
> Albert ARIBAUD wrote:
> > Most probably 2); mentioning a file in the linker script either with or
> > without mentioning it on the command line has certainly been done for
> > some time, so I doubt the feature is new; and certainly the doc is about
> > files, not symbols in different files.
> 
> There seems to be a (here significant) difference between object files
> on the command line and libraries (which appear to be handled like
> a mere collection of object files).

the difference is that the linker wont bother looking at the duplicated 
objects in the archives.  from the linker's perspective though, we now have to 
completely independent objects.  the combined one specified on the command 
line and the split up ones in the linker script.

one possible way to fix boards is to stop specifying sub-objects in the linker 
script and only specify the combined ones.  so in board/tqc/tqm8xx/u-boot.lds, 
drop the split objects like lib/zlib.o in favor of the combined one like 
lib/libgeneric.o.  this might not work for everyone since the combined object 
sizes can be a bit large.  if everyone was using -ffunction-sections/-fdata-
sections 

this however wont work for common/env_embedded.o since it is merged 
common/libcommon.o.  so to fix that, we'll need to not merge env_embedded.o 
into libcommon.o.  like in the patch below.
-mike

Comments

Wolfgang Denk Nov. 20, 2010, 8:38 a.m. UTC | #1
Dear Mike Frysinger,

In message <201011191908.00162.vapier@gentoo.org> you wrote:
>
> the difference is that the linker wont bother looking at the duplicated 
> objects in the archives.  from the linker's perspective though, we now have to 
> completely independent objects.  the combined one specified on the command
> line and the split up ones in the linker script.

Right.

> one possible way to fix boards is to stop specifying sub-objects in the linker 
> script and only specify the combined ones.  so in board/tqc/tqm8xx/u-boot.lds, 
> drop the split objects like lib/zlib.o in favor of the combined one like 
> lib/libgeneric.o.  this might not work for everyone since the combined object 

It doesn't work as we cannot fine-adjust the size of the combined
objects as it's needed for at least coarse adjustment to the available
flash sector sizes.

> sizes can be a bit large.  if everyone was using -ffunction-sections/-fdata-
> sections 

?  Could you please complete that sentence?

> this however wont work for common/env_embedded.o since it is merged 
> common/libcommon.o.  so to fix that, we'll need to not merge env_embedded.o
> into libcommon.o.  like in the patch below.
> -mike
>
> diff --git a/common/Makefile b/common/Makefile
> index e0db382..d38aa7b 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -52,10 +52,10 @@ COBJS-y += cmd_version.o
>  COBJS-y += env_common.o
>  COBJS-$(CONFIG_ENV_IS_IN_DATAFLASH) += env_dataflash.o
>  COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += env_eeprom.o
> -COBJS-$(CONFIG_ENV_IS_EMBEDDED) += env_embedded.o
> -COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += env_embedded.o
> -COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_embedded.o
> -COBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_embedded.o
> +XOBJS-$(CONFIG_ENV_IS_EMBEDDED) += env_embedded.o
> +XOBJS-$(CONFIG_ENV_IS_IN_EEPROM) += env_embedded.o
> +XOBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_embedded.o
> +XOBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_embedded.o
>  COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o
>  COBJS-$(CONFIG_ENV_IS_IN_MG_DISK) += env_mgdisk.o
>  COBJS-$(CONFIG_ENV_IS_IN_MMC) += env_mmc.o

Is this not too much? We have embedded environment only in the case
of NOR flash, so the extra handling should only be done for
CONFIG_ENV_IS_EMBEDDED ?

Best regards,

Wolfgang Denk
Albert ARIBAUD Nov. 20, 2010, 8:48 a.m. UTC | #2
Le 20/11/2010 09:38, Wolfgang Denk a écrit :
> Dear Mike Frysinger,
>
> In message<201011191908.00162.vapier@gentoo.org>  you wrote:

>> sizes can be a bit large.  if everyone was using -ffunction-sections/-fdata-
>> sections
>
> ?  Could you please complete that sentence?

I believe the idea is that with these options, the build produces not 
one single input section for all code and one single input section for 
all data of a given object file, but one input section per function and 
one input section per variable, thus allowing the linker file to 
fin-grain-tune the content of the isolated sector(s), specifying exactly 
which function or (initialized) data would go in there.

Amicalement,
Mike Frysinger Nov. 20, 2010, 9:19 a.m. UTC | #3
On Saturday, November 20, 2010 03:38:44 Wolfgang Denk wrote:
> Mike Frysinger wrote:
> > one possible way to fix boards is to stop specifying sub-objects in the
> > linker script and only specify the combined ones.  so in
> > board/tqc/tqm8xx/u-boot.lds, drop the split objects like lib/zlib.o in
> > favor of the combined one like lib/libgeneric.o.  this might not work
> > for everyone since the combined object
> 
> It doesn't work as we cannot fine-adjust the size of the combined
> objects as it's needed for at least coarse adjustment to the available
> flash sector sizes.
> 
> > sizes can be a bit large.  if everyone was using
> > -ffunction-sections/-fdata- sections
> 
> ?  Could you please complete that sentence?

if everyone used these options, then the linker script would get fine grained 
control as people could still specify the combined object, but only pull in 
specific sections.  so the linker script could do in the leading space:
	lib/libgeneric.o (.text.crc32 .text.gunzip .text........)
and then in the normal space after the env, do:
	lib/libgeneric.o (.text*)
since the linker has already placed the specific sections earlier, the glob 
wont pull them in again.

while these options normally imply the end goal of --gc-sections, using that 
flag isnt a requirement.  as long as the linker scripts specify things like 
".text*" instead of just ".text", then they should work fine.  and people dont 
have to worry about the linker discarding unreferenced sections (such as reset 
vectors or whatever).  at least not today ... this really should get fixed 
across the board.

> > this however wont work for common/env_embedded.o since it is merged
> > common/libcommon.o.  so to fix that, we'll need to not merge
> > env_embedded.o into libcommon.o.  like in the patch below.
> > 
> > diff --git a/common/Makefile b/common/Makefile
> > index e0db382..d38aa7b 100644
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -52,10 +52,10 @@ COBJS-y += cmd_version.o
> > 
> >  COBJS-y += env_common.o
> >  COBJS-$(CONFIG_ENV_IS_IN_DATAFLASH) += env_dataflash.o
> >  COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += env_eeprom.o
> > 
> > -COBJS-$(CONFIG_ENV_IS_EMBEDDED) += env_embedded.o
> > -COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += env_embedded.o
> > -COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_embedded.o
> > -COBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_embedded.o
> > +XOBJS-$(CONFIG_ENV_IS_EMBEDDED) += env_embedded.o
> > +XOBJS-$(CONFIG_ENV_IS_IN_EEPROM) += env_embedded.o
> > +XOBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_embedded.o
> > +XOBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_embedded.o
> > 
> >  COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o
> >  COBJS-$(CONFIG_ENV_IS_IN_MG_DISK) += env_mgdisk.o
> >  COBJS-$(CONFIG_ENV_IS_IN_MMC) += env_mmc.o
> 
> Is this not too much? We have embedded environment only in the case
> of NOR flash, so the extra handling should only be done for
> CONFIG_ENV_IS_EMBEDDED ?

dont really know what you're referring to ... this patch works on my Blackfin 
boards.  however, it doesnt really matter, as another option would be to 
simply stick the env into its own sections.  two ways to accomplish this:

(1) if we use -ffunction-sections/-fdata-sections for everyone and we punt the 
__PPCENV__ and __PPCTEXT__ hacks, in the linker script, you could do:
	common/libcommon.o (.data.environment .data.redundand_environment 
.data.env_size)

(2) or tweak/extend the __PPCENV__ and __PPCTEXT__ hacks to manually place the 
vars into more specific sections rather than just ".text".  then the linker 
script would do something like:
	common/libcommon.o (.text.environment .text.redundand_environment 
.text.env_size)

obviously i'd prefer (1) since the gcc attribute usage in env_embedded.c makes 
me want to barf.
-mike
diff mbox

Patch

diff --git a/common/Makefile b/common/Makefile
index e0db382..d38aa7b 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -52,10 +52,10 @@  COBJS-y += cmd_version.o
 COBJS-y += env_common.o
 COBJS-$(CONFIG_ENV_IS_IN_DATAFLASH) += env_dataflash.o
 COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += env_eeprom.o
-COBJS-$(CONFIG_ENV_IS_EMBEDDED) += env_embedded.o
-COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += env_embedded.o
-COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_embedded.o
-COBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_embedded.o
+XOBJS-$(CONFIG_ENV_IS_EMBEDDED) += env_embedded.o
+XOBJS-$(CONFIG_ENV_IS_IN_EEPROM) += env_embedded.o
+XOBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_embedded.o
+XOBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_embedded.o
 COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o
 COBJS-$(CONFIG_ENV_IS_IN_MG_DISK) += env_mgdisk.o
 COBJS-$(CONFIG_ENV_IS_IN_MMC) += env_mmc.o
@@ -174,7 +174,7 @@  OBJS	:= $(addprefix $(obj),$(AOBJS) $(COBJS))
 
 CPPFLAGS += -I..
 
-all:	$(LIB) $(AOBJS)
+all:	$(LIB) $(AOBJS) $(XOBJS-y)
 
 $(LIB): $(obj).depend $(OBJS)
 	$(call cmd_link_o_target, $(OBJS))