Re: [RFC] Optimize __arch_swab32 and __arch_swab16

Joakim Tjernlund [off-list ref] writes:

unsigned short my__arch_swab16(unsigned short value)
{
	__asm__("rlwimi %0,%0,16,0x00ff0000"
		: "+r" (value));

You are creating a value that does not fit in a short.

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

Andreas Schwab [off-list ref] wrote on 2011/08/11 10:45:42:

Joakim Tjernlund [off-list ref] writes:

quoted

unsigned short my__arch_swab16(unsigned short value)
{
   __asm__("rlwimi %0,%0,16,0x00ff0000"
      : "+r" (value));

You are creating a value that does not fit in a short.

Short is passed in a 32 bit register with the upper 16 bits cleared. I just
temporarily use the upper bits and shift it back with the next insn:
__asm__("rlwinm %0,%0,24,0x0000ffff"
    : "+r"(value));
Can I not use the upper 16 bits in this manner?

 Jocke

Joakim Tjernlund [off-list ref] writes:
=20

quoted

unsigned short my__arch_swab16(unsigned short value)
{
	__asm__("rlwimi %0,%0,16,0x00ff0000"
		: "+r" (value));

=20
You are creating a value that does not fit in a short.

Which is a problem because the compiler could schedule
it be written back to real memory between the instructions.

Actually the generated code would be better if the swap16()
functions operated on 'unsigned int' fields - since it would
save the compiler from doing a lot of shifts/masks elsewhere.

For instance there is likely to be a mask with 0xffff prior
to the call to swab16().

Since one use of these is for htons() (etc), when
the host is the correct endianness these are #defines that
do nothing - so out of range values aren't masked.
So it seems to me that defining:
   unsigned int swab(unsigned int);
would be fine - except it clashes with standard headers :-(

	David

"David Laight" [off-list ref] wrote on 2011/08/11 10:56:26:

quoted

Joakim Tjernlund [off-list ref] writes:

quoted

unsigned short my__arch_swab16(unsigned short value)
{
   __asm__("rlwimi %0,%0,16,0x00ff0000"
      : "+r" (value));

You are creating a value that does not fit in a short.

Which is a problem because the compiler could schedule
it be written back to real memory between the instructions.

It can? There is no memory here, just registers. Even if it
is written to memory, how would that affect the register?

Assuming you are right, would rewriting it to
  __asm__("rlwimi %0,%0,16,0x00ff0000\n\t"
	    "rlwinm %0,%0,24,0x0000ffff"
	    : "+r"(value));
help?

Actually the generated code would be better if the swap16()
functions operated on 'unsigned int' fields - since it would
save the compiler from doing a lot of shifts/masks elsewhere.

For instance there is likely to be a mask with 0xffff prior
to the call to swab16().

Since one use of these is for htons() (etc), when
the host is the correct endianness these are #defines that
do nothing - so out of range values aren't masked.
So it seems to me that defining:
   unsigned int swab(unsigned int);
would be fine - except it clashes with standard headers :-(

   David

=20

quoted

Which is a problem because the compiler could schedule
it be written back to real memory between the instructions.

=20
It can? There is no memory here, just registers. Even if it
is written to memory, how would that affect the register?

Although the function argument is passed in a register, the
compiler could generate a store-load sequence before and
after each __asm__() line.

Assuming you are right, would rewriting it to
  __asm__("rlwimi %0,%0,16,0x00ff0000\n\t"
	    "rlwinm %0,%0,24,0x0000ffff"
	    : "+r"(value));
help?

Except that now you've stopped the compiler scheduling
another instruction between the two - probably forcing a
execution stall.

	David

"David Laight" [off-list ref] wrote on 2011/08/11 11:29:33:

quoted

quoted

Which is a problem because the compiler could schedule
it be written back to real memory between the instructions.

It can? There is no memory here, just registers. Even if it
is written to memory, how would that affect the register?

Although the function argument is passed in a register, the
compiler could generate a store-load sequence before and
after each __asm__() line.

Ah, I see. Seems strange that the complier would do that for
the register in use(value). Other regs perhaps.

quoted

Assuming you are right, would rewriting it to
  __asm__("rlwimi %0,%0,16,0x00ff0000\n\t"
       "rlwinm %0,%0,24,0x0000ffff"
       : "+r"(value));
help?

Except that now you've stopped the compiler scheduling
another instruction between the two - probably forcing a
execution stall.

But this should be better than using 2 more insns and an
extra register.

 Jocke