ARM missing __udivdi3 in lib_arm or fix 64bit division in nand_util.c?
On ARM (don't know for other architectures ;) ) compiling and linking nand_util.c results on recent git in
~/uboot/drivers/nand/nand_util.c:657: undefined reference to __udivdi3' drivers/nand/libnand.a(nand_util.o): In function nand_write_opts':~/uboot/drivers/nand/nand_util.c:481: undefined reference to __udivdi3' drivers/nand/libnand.a(nand_util.o): In function nand_erase_opts':~/uboot/drivers/nand/nand_util.c:214: undefined reference to `__udivdi3'
In lib_arm __udivsi3 and friends are available, but __udivdi3 is missing. There is a fix by modifying nand_util.c
to not do any 64bit divisions any more. Now, I wonder what is the correct fix for this? Should lib_arm provide __udivdi3 as well or should nand_util.c be fixed as in above link to avoid 64bit divisions?
Best regards
Dirk
Quoted 8 lines · Dirk Behme
In
lib_arm__udivsi3and friends are available, but__udivdi3is missing. There is a fix by modifyingnand_util.cto not do any 64bit divisions any more. Now, I wonder what is the correct fix for this? Should
lib_armprovide__udivdi3as well or shouldnand_util.cbe fixed as in above link to avoid 64bit divisions?
I can't speak for ARM, but I believe this is a problem for most architectures. In general, I think we should seriously consider moving lib_avr32/div64.c into lib_generic and start to use it for 64-bit division instead of relying on libgcc. U-Boot NG has already done this.
In this particular case, I think it's just ridiculously expensive to do _three_ 64-bit divisions just to implement a simple progress bar and they should all go away. It looks like Patrice's patch might give a bit weird results when crossing a 4G boundary but I don't see any easy way around it. Perhaps we should just drop the whole percentage complete thing and just show a nice N / M fraction?
H?vard
Quoted 11 lines · H?vard Skinnemoen
Quoted 8 lines · Dirk Behme
In
lib_arm__udivsi3and friends are available, but__udivdi3is missing. There is a fix by modifyingnand_util.cto not do any 64bit divisions any more. Now, I wonder what is the correct fix for this? Should
lib_armprovide__udivdi3as well or shouldnand_util.cbe fixed as in above link to avoid 64bit divisions?
Quoted 5 lines
I can't speak for ARM, but I believe this is a problem for most architectures. In general, I think we should seriously consider moving
lib_avr32/div64.cintolib_genericand start to use it for 64-bit division instead of relying on libgcc. U-Boot NG has already done this.
Something like in attachment?
Quoted 6 lines
In this particular case, I think it's just ridiculously expensive to do _three_ 64-bit divisions just to implement a simple progress bar and they should all go away. It looks like Patrice's patch might give a bit weird results when crossing a 4G boundary but I don't see any easy way around it. Perhaps we should just drop the whole percentage complete thing and just show a nice N / M fraction?
Sounds good! Do you have some code for this?
Dirk
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: div64_generic_patch.txt
Url: http://lists.denx.de/pipermail/u-boot/attachments/20070731/30b15fe6/attachment.txt
Quoted 2 lines · In message <[email protected]> you wrote:
Something like in attachment?
Probably not.
Quoted 4 lines
(quoted diff) +4
-0+/* The unnecessary pointer compare is there + * to check for type safety (n must be 64bit) + */ +# define do_div(n,base) ({ \
Review
Quoted 10 lines
(quoted diff) +10
-0+ uint32_t __base = (base); \ + uint32_t __rem; \ + (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ + if (((n) >> 32) == 0) { \ + __rem = (uint32_t)(n) % __base; \ + (n) = (uint32_t)(n) / __base; \ + } else \ + __rem = __div64_32(&(n), __base); \ + __rem; \ + })
CodingStyle: Generally, inline functions are preferable to macros resembling functions.
lib_generic/Makefile LIB = $(obj)libgeneric.a · line 31
Quoted 11 lines
Index: uboot/lib_generic/Makefile
===================================================================lib_generic/Makefile +1
-1@@ -27,7 +27,7 @@ LIB = $(obj)libgeneric.a COBJS = bzlib.o bzlib_crctable.o bzlib_decompress.o \ bzlib_randtable.o bzlib_huffman.o \ - crc32.o ctype.o display_options.o ldiv.o sha1.o \ + crc32.o ctype.o display_options.o div64.o ldiv.o sha1.o \ string.o vsprintf.o zlib.o
Why should I link this code and increase the memory footprint for all boards, when 99% of them don't need this?
Rejected.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "We Americans, we're a simple people... but piss us off, and we'll bomb your cities." - Robin Williams, _Good Morning Vietnam_
Quoted 5 lines · Wolfgang Denk
Quoted 1 line · In message <[email protected]> you wrote:
Something like in attachment?
Probably not.
Thanks for commenting!
Quoted 10 lines
Quoted 10 lines
(quoted diff) +10
-0+/* The unnecessary pointer compare is there + * to check for type safety (n must be 64bit) + */ +# define do_div(n,base) ({ \ + uint32_t __base = (base); \ + uint32_t __rem; \ + (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ + if (((n) >> 32) == 0) { \ + __rem = (uint32_t)(n) % __base; \ + (n) = (uint32_t)(n) / __base; \
Quoted 8 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ } else \ + __rem = __div64_32(&(n), __base); \ + __rem; \ + })CodingStyle: Generally, inline functions are preferable to macros resembling functions.
I know that this is no excuse for bad coding style, but please note that this stuff is already part of U-Boot, see lib_avr32/div64.c and include/asm-avr32/div64.h. My patch only moves the *unmodfied* files to lib_generic for general use, as proposed by H?vard. Same as U-Boot NG did ;)
Quoted 6 lines
Quoted 6 lines
Index: uboot/lib_generic/Makefile
===================================================================lib_generic/Makefile +0
-0@@ -27,7 +27,7 @@ LIB = $(obj)libgeneric.a
Quoted 9 lines
lib_generic/Makefile LIB = $(obj)libgeneric.a · line 31
Quoted 5 lines
lib_generic/Makefile +1
-1COBJS = bzlib.o bzlib_crctable.o bzlib_decompress.o \ bzlib_randtable.o bzlib_huffman.o \ - crc32.o ctype.o display_options.o ldiv.o sha1.o \ + crc32.o ctype.o display_options.o div64.o ldiv.o sha1.o \ string.o vsprintf.o zlib.oWhy should I link this code and increase the memory footprint for all boards, when 99% of them don't need this?
Sorry if I misunderstood anything here. But with putting do_div/__div64_32 to a *library* boards use it only if they need it? Here, if they explicitly use do_div()? If they don't use do_div(), the memory footprint isn't increased because it simply isn't used?
Please find below some tests with omap5912osk_config which doesn't use do_div/__div64_32. The first numbers are with clean recent git, the second with my patch with div64 stuff moved to lib_generic. As expected, library size increases but image size stays the same. Anything wrong with this?
Quoted 1 line
Rejected.
What's your proposal then with the 64bit division issue in nand_util.c?
Best regards
Dirk
==> Clean recent git:
Quoted 3 lines
make
omap5912osk_config
make
ll u-boot.bin
... 100016 ... u-boot.bin
Quoted 1 line
ll lib_generic/libgeneric.a
... 102740 ... lib_generic/libgeneric.a
Quoted 1 line
arm-elf-ar -t lib_generic/libgeneric.a
bzlib.o
bzlib_crctable.o
bzlib_decompress.o
bzlib_randtable.o
bzlib_huffman.o
crc32.o
ctype.o
display_options.o
ldiv.o
sha1.o
string.o
vsprintf.o
zlib.o
Quoted 1 line
==> Clean recent git + div64_generic_patch.txt:
Quoted 3 lines
make
omap5912osk_config
make
ll u-boot.bin
... 100016 ... u-boot.bin
Quoted 1 line
ll lib_generic/libgeneric.a
... 106208 ... lib_generic/libgeneric.a
Quoted 1 line
arm-elf-ar -t lib_generic/libgeneric.a
bzlib.o
bzlib_crctable.o
bzlib_decompress.o
bzlib_randtable.o
bzlib_huffman.o
crc32.o
ctype.o
display_options.o
div64.o
ldiv.o
sha1.o
string.o
vsprintf.o
zlib.o
Quoted 1 line
Quoted 9 lines · In message <[email protected]> you wrote:
Quoted 3 lines
CodingStyle: Generally, inline functions are preferable to macros resembling functions.
I know that this is no excuse for bad coding style, but please note that this stuff is already part of U-Boot, see
lib_avr32/div64.candinclude/asm-avr32/div64.h. My patch only moves the *unmodfied* files tolib_genericfor general use, as proposed by H?vard. Same as U-Boot NG did ;)
Yes, I understood thos. Just thought I might talk you into even more improvements :-)
Quoted 10 lines
Quoted 9 lines
Quoted 5 lines
COBJS = bzlib.o bzlib_crctable.o bzlib_decompress.o \ bzlib_randtable.o bzlib_huffman.o \(quoted diff) +1
-1- crc32.o ctype.o display_options.o ldiv.o sha1.o \ + crc32.o ctype.o display_options.o div64.o ldiv.o sha1.o \string.o vsprintf.o zlib.o
Why should I link this code and increase the memory footprint for all boards, when 99% of them don't need this?
Quoted 2 lines
Sorry if I misunderstood anything here. But with putting do_div/__div64_32 to a *library* boards use it only if they need it?
Right. Objects from a librariry get only pulled into the linked image when they are referenced somewhere. Objects explicitely listed on the command line get always included.
I failed to see that this was an object list for another library. Sorry for the false alarm.
Quoted 1 line
Anything wrong with this?
No. I was wrong.
Quoted 3 lines
Quoted 1 line
Rejected.
What's your proposal then with the 64bit division issue in
nand_util.c?
I withdraw the reject.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Sometimes a feeling is all we humans have to go on. -- Kirk, "A Taste of Armageddon", stardate 3193.9
Quoted 2 lines · In message <[email protected]> you wrote:
What's your proposal then with the 64bit division issue in
nand_util.c?
Please repost the patch in correct form, i. e. with Signed-off-by:
line added... the merge window is open.
Best regards,
Wolfgang Denk
Signature · 4 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Madness takes its toll. Please have exact change.