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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox