On 05/07/2013 08:18 PM, Steven J. Hill wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 05/07/2013 06:01 PM, David Daney wrote:
On 12/06/2012 09:05 PM, Steven J. Hill wrote:
From: "Steven J. Hill" <sjhill@xxxxxxxx>
Optimise 'strncpy' to use microMIPS instructions and/or optimisations for
binary size reduction. When the microMIPS ISA is not being used, the
library function compiles to the original binary code.
This is an untrue statement. Why mislead us by saying the original binary
code is obtained?
I you are building a classic MIPS kernel, the instructions generated will be
the same even with this patch. The changes only make a difference when
building a pure microMIPS kernel.
You are wrong:
--- strncpy_user.o.before.dis 2013-05-08 09:14:35.895555668 -0700
+++ strncpy_user.o.after.dis 2013-05-08 09:14:12.870485085 -0700
@@ -1,5 +1,5 @@
-strncpy_user.o.before: file format elf64-tradbigmips
+strncpy_user.o.after: file format elf64-tradbigmips
Disassembly of section .text:
@@ -7,27 +7,26 @@
0000000000000000 <__strncpy_from_user_asm>:
0: df820028 ld v0,40(gp)
4: 00451024 and v0,v0,a1
- 8: 14400011 bnez v0,50 <__strncpy_from_user_nocheck_asm+0x40>
+ 8: 14400010 bnez v0,4c <__strncpy_from_user_nocheck_asm+0x3c>
c: 00000000 nop
0000000000000010 <__strncpy_from_user_nocheck_asm>:
- 10: 0000102d move v0,zero
+ 10: 0000602d move t0,zero
14: 00a0182d move v1,a1
- 18: 906c0000 lbu t0,0(v1)
+ 18: 90620000 lbu v0,0(v1)
1c: 64630001 daddiu v1,v1,1
- 20: 11800005 beqz t0,38 <__strncpy_from_user_nocheck_asm+0x28>
- 24: a08c0000 sb t0,0(a0)
- 28: 64420001 daddiu v0,v0,1
- 2c: 64840001 daddiu a0,a0,1
- 30: 1446fff9 bne v0,a2,18 <__strncpy_from_user_nocheck_asm+0x8>
- 34: 00000000 nop
- 38: 00a2602d daddu t0,a1,v0
- 3c: 01856026 xor t0,t0,a1
- 40: 05800003 bltz t0,50 <__strncpy_from_user_nocheck_asm+0x40>
- 44: 00000000 nop
- 48: 03e00008 jr ra
- 4c: 00000000 nop
- 50: 2402fff2 li v0,-14
- 54: 03e00008 jr ra
- 58: 00000000 nop
- 5c: 00000000 nop
+ 20: 10400004 beqz v0,34 <__strncpy_from_user_nocheck_asm+0x24>
+ 24: a0820000 sb v0,0(a0)
+ 28: 658c0001 daddiu t0,t0,1
+ 2c: 1586fffa bne t0,a2,18 <__strncpy_from_user_nocheck_asm+0x8>
+ 30: 64840001 daddiu a0,a0,1
+ 34: 00ac102d daddu v0,a1,t0
+ 38: 00451026 xor v0,v0,a1
+ 3c: 04400003 bltz v0,4c <__strncpy_from_user_nocheck_asm+0x3c>
+ 40: 00000000 nop
+ 44: 03e00008 jr ra
+ 48: 0180102d move v0,t0
+ 4c: 2402fff2 li v0,-14
+ 50: 03e00008 jr ra
+ 54: 00000000 nop
+ ...
They are different, and you said they would be the same.
I am fine if you want to change things. Just don't say that your patch
makes no change when it in fact does.
You don't really explain how the change helps optimization either.
The exercise is left to the reader. Build a microMIPS kernel yourself and
figure it out.
This isn't some sort of programming text book. Your job in the change
log (and the mailing list) isn't to force us to learn by doing a lot of
independent analysis of the code. Instead I would prefer a concise
explanation of why the change is beneficial.
You are dumping a lot of new code into the kernel. That is fine, but
you could consider making the process easier by improving the quality of
the changelogs that accompany it.
David Daney
- -Steve
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlGJxBcACgkQgyK5H2Ic36c4hQCeLGI8MI2rr6KgOv7G15lnBdok
bbcAoKY+BvVyTCzG033Bc+pJ07xCtGMq
=xJmM
-----END PGP SIGNATURE-----
