Re: bug report

[Zack Weinberg <zack at wolery dot cumb dot org>]

[...]
> As you can see, the compiler ignores the `unsigned res = 0' statement.
> That's not a good idea because the code above works properly only with a
> `len' argument of 32. My work around is inserting an additional
> statement `xorl %0,%0' just before the `rev1:' label.

Thank you for your bug report.  This is an error in the constraints on
your asm statement.  gcc thinks that the value initially stored in
'res' is not used, so it discards it.  You must indicate that the asm
reads the value of 'res' as well as writes it.

With gcc 2.95, you can write

  asm ("decl %1
rev1:
        rcrl $1,%2
        rcll $1,%0
        loop rev1" : "+r" (res) : "c" (len), "d" (code));

- the '=' in the output is changed to a '+'.  This produces the code

reverse_bits:
        pushl %ebx
        movl %eax,%ebx
        movl %edx,%ecx
        xorl %eax,%eax
        movl %ebx,%edx
#APP
        decl %ecx
rev1:
        rcrl $1,%edx
        rcll $1,%eax
        loop rev1
#NO_APP
        popl %ebx
        ret

which is correct, although it does more register shuffling than is
strictly necessary.  I do not believe that you could use '+' in asm
constraints with 1.1.2; an equivalent construct which works with that
version is

  asm ("decl %2
rev1:
        rcrl $1,%3
        rcll $1,%0
        loop rev1" : "=r" (res) : "0" (res), "c" (len), "d" (code));

Two additional notes: you should use a 'local label', so you can
inline this function safely, and you should let the compiler do as
much of the register allocation as possible.  The only instruction
that cares which register something's in is 'loop', so:

  asm ("decl %2
1:
        rcrl $1,%3
        rcll $1,%0
        loop 1b" : "=r" (res) : "0" (res), "c" (len), "r" (code));


That gets you this assembly:

reverse_bits:
        pushl   %ebx
        movl    %edx, %ecx
        movl    %eax, %ebx
        xorl    %eax, %eax
#APP
        decl %ecx
1:
        rcrl $1,%ebx
        rcll $1,%eax
        loop 1b
#NO_APP
        popl    %ebx
        ret
.Lfe1:

It is still not perfect - I don't know why it insists on using ebx.
(The register allocator has issues. :P)

zw