lkml.org 
[lkml]   [2009]   [Feb]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2 resend] libata-sff: avoid byte swapping in ata_sff_data_xfer()
Hello, I wrote:

>>> Handling of the trailing byte in ata_sff_data_xfer() is suboptimal
>>> bacause:

>>> - it always initializes the padding buffer to 0 which is not really
>>> needed in
>>> both the read and write cases;

>>> - it has to use memcpy() to transfer a single byte from/to the
>>> padding buffer;

>> Have you looked at the assembly, before deciding it is suboptiomal?

> I'm estimating the code itself, not what the compiler can do to fix
> it. :-)

>> gcc optimizes tiny arrays and structures quite well, and is well
>> capable of seeing one path where the initialization is clobbered
>> without a single read, and another code path where it is used.

> The initialier just shouldn't have been there in the first place,
> clobbered or not. And let's looks at what gcc gave me:

[...]

>> As for memcpy, for small and/or constant values that is quite often a
>> compiler builtin. It is rarely useful, these days, to convert a
>> memcpy() to a hand-rolled

> version of same.

> Here memcpy() just shouldn't have appeared in the first place. But
> indeed, gcc did optimize it away.

In fact, we could do without both memcpy and io*15_rep() I think:

if (unlikely(buflen & 0x01)) {
u16 pad;

/* Point buf to the tail of buffer */
buf += buflen - 1;

/*
* Copy from/to pad's LSB only (host order),
* dropping its MSB or zero-extending it...
*/
if (rw == READ) {
pad = ioread16(data_addr);
*buf = (unsigned char)pad;
} else {
pad = *buf;
iowrite16(pad, data_addr);
}
}

It should work -- that easy... although io{read|write}16() will still
byte-swap.

>> Jeff

MBR, Sergei


\
 
 \ /
  Last update: 2009-02-18 12:23    [W:0.148 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site