From: David Daney <ddaney.cavm@gmail.com>
To: "Steven J. Hill" <sjhill@realitydiluted.com>
Cc: linux-mips@linux-mips.org
Subject: Re: [PATCH v99,11/13] MIPS: microMIPS: Optimise 'strncpy' core library function.
Date: Wed, 08 May 2013 09:28:48 -0700 [thread overview]
Message-ID: <518A7D40.1060502@gmail.com> (raw)
In-Reply-To: <5189C41D.3000005@realitydiluted.com>
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@mips.com>
>>>
>>> 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-----
>
>
>
next prev parent reply other threads:[~2013-05-08 16:29 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-07 5:05 [PATCH v99,00/13] Add support for pure microMIPS kernel Steven J. Hill
2012-12-07 5:05 ` [PATCH v99,01/13] MIPS: microMIPS: Add support for microMIPS instructions Steven J. Hill
2012-12-07 7:50 ` Kevin Cernekee
2012-12-07 15:24 ` Ralf Baechle
2012-12-15 5:15 ` Hill, Steven
2012-12-07 5:05 ` [PATCH v99,02/13] MIPS: Whitespace clean-ups after microMIPS additions Steven J. Hill
2012-12-07 7:59 ` Kevin Cernekee
2012-12-07 5:05 ` [PATCH v99,03/13] MIPS: microMIPS: Floating point support for 16-bit instructions Steven J. Hill
2012-12-07 5:05 ` [PATCH v99,04/13] MIPS: microMIPS: Add support for exception handling Steven J. Hill
2012-12-07 5:05 ` [PATCH v99,05/13] MIPS: microMIPS: Support handling of delay slots Steven J. Hill
2012-12-07 5:05 ` [PATCH v99,06/13] MIPS: microMIPS: Add unaligned access support Steven J. Hill
2012-12-07 5:05 ` [PATCH v99,07/13] MIPS: microMIPS: Add vdso support Steven J. Hill
2012-12-07 5:05 ` [PATCH v99,08/13] MIPS: microMIPS: Add configuration option for microMIPS kernel Steven J. Hill
2012-12-07 5:05 ` [PATCH v99,09/13] MIPS: microMIPS: Work-around for assembler bug Steven J. Hill
2012-12-07 5:05 ` [PATCH v99,10/13] MIPS: microMIPS: Optimise 'memset' core library function Steven J. Hill
2012-12-07 5:05 ` [PATCH v99,11/13] MIPS: microMIPS: Optimise 'strncpy' " Steven J. Hill
2013-05-07 23:01 ` David Daney
2013-05-08 3:18 ` Steven J. Hill
2013-05-08 16:28 ` David Daney [this message]
2013-05-18 23:25 ` Maciej W. Rozycki
2012-12-07 5:05 ` [PATCH v99,12/13] MIPS: microMIPS: Optimise 'strlen' " Steven J. Hill
2012-12-07 5:05 ` [PATCH v99,13/13] MIPS: microMIPS: Optimise 'strnlen' " Steven J. Hill
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=518A7D40.1060502@gmail.com \
--to=ddaney.cavm@gmail.com \
--cc=linux-mips@linux-mips.org \
--cc=sjhill@realitydiluted.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.