RE: [avr-libc-dev] Fix for strstr.S (revision 2)

[Philip Søeberg]

>>But then i took a closer look, and optimized it a little. ;-)
A little? :D Great work... 

>>To begin, i've been embarrassed about the complexity of the new version
Please don’t feel embarrassed by other ppls work... We can't all be as good
as you may be :) .. I didn't say that v2 of the code was particular
beautiful or fast, but at least it worked...

The one-char xjmp thingy was actually a leftover from version1 that I did
the day before... Didn't clean the code properly.. My bad :(

Anyway... I can see that I still have much to learn, and must begin tearing
more modules apart :) Thanks for the great work :)

Regards.
-=[ Philip Soeberg ]=[ address@hidden ]=-

-----Original Message-----
From: address@hidden
[mailto:address@hidden On Behalf Of
Joerg Wunsch
Sent: 10. december 2002 22:43
To: avr-libc-dev
Subject: Re: [avr-libc-dev] Fix for strstr.S (revision 2)

As Theodore A. Roth wrote:

> Attached is what I'll probably commit to cvs.

OK, here's my version. ;-)

To begin, i've been embarassed about the complexity of the new
version...  I was tempted to just pick the C implementation from the
BSD libc, and see, the resulting binary was just four bytes larger
(only) than the assembler version.  Given that the BSD implementation
uses strncmp() and strlen(), and there's a good chance that an
application using strstr() is already going to use at least one of
both, the C version would even have saved some space.

But then i took a closer look, and optimized it a little. ;-)

> I took out the code to check for a single char string then call
> strchr().

I noticed that, too.  I didn't understand what the special check is
good for, and just dropped it.  See, it still works. :)

> It turns out that gcc does that for you automagically when you turn on
> optimizations, but it doesn't if you don't.

How would optimization optimize an assembler file?

Anyway, the one-char string is just a pretty good generic C string as
well, so the generic handling that applies to all other strings
applies to it very much the same.

Then i dropped all the useless push/pop instructions.  r18 through r27
are call-used registers that may be clobbered by any function.  See
http://savannah.nongnu.org/download/avr-libc/doc/avr-libc-user-manual/FAQ.ht
ml#faq_reg_usage

I finally optimized a few other things.  The various #defines are
well-suited to obfuscate that the first incoming paramater and the
return value of a function are identical, so the assignment in case of
a second null string was just an expensive no-op.  Dropped, some more
bytes saved. :)

Since the stack pops are gone, there's no point in using XJMP to jump
to a single ret instruction.  Just use ret directly.  Some more bytes
saved. :)

The resulting binary is about 30 bytes smaller than Philip's version
(on an avr5 architecture).  For avr5, a further instruction could be
saved by using movw to assing X to ret_{lo,hi}, but since this would
have caused another preprocessor conditional not already covered by
macros.inc, i decided that two saved bytes weren't worth it.

Finally, i also re-added the redistribution notice (as Ted did).  A
file without this notice is implicitly not freely redistributable,
thus we need to have the notice.

Since this is effectively a complete rewrite, and there's thus no need
to adhere to the (sometimes unusual) style of the old file, i also
changed some stylistic things:

. use no spaces but a tab to indent the opcode
. use a tab to separate the operands from the opcode (both are usual
  assembler style)
. use assembler comments instead C++ comments (helps syntax highlighting,
  and is simply more common for assembler files)
. use true local labels that won't appear in the linker symbol table
  (starting with .L), this also allows to reduce the label name length
  since there's no risk of a global name clash

I hope you guys don't mind all this. ;-)

> I did notice that your test case is missing a check to make sure that the
> returned value is the expected pointer. I've also attached a program which
> also tests this.

That's fine.  I also noticed the omission, but just verified in
simulavr that the returned pointers are what they ought to be.  Of
course, verifying this in the test suite is better.  I agree with Eric
that it's time to start an official test suite...

-- 
J"org Wunsch                                           Unix support engineer
address@hidden        http://www.interface-systems.de/~j/