Re: [PATCH 1/1] aacraid: big endian issues
On Jan 8, 2008 12:48 PM, Salyzyn, Mark [EMAIL PROTECTED] wrote:
Big endian systems issues discovered in the aacraid driver.
...
--- a/drivers/scsi/aacraid/comminit.c 2008-01-08 15:32:28.329810853 -0500
+++ b/drivers/scsi/aacraid/comminit.c 2008-01-08 15:37:35.633163607 -0500
@@ -301,10 +301,10 @@
if ((!aac_adapter_sync_cmd(dev, GET_ADAPTER_PROPERTIES,
0, 0, 0, 0, 0, 0, status+0, status+1, status+2, NULL, NULL))
(status[0] == 0x0001)) {
- if (status[1] AAC_OPT_NEW_COMM_64)
+ if (status[1] le32_to_cpu(AAC_OPT_NEW_COMM_64))
...
Why apply le32_to_cpu() to the constant instead of the variable?
On systems were le32_to_cpu() is doing something, can gcc or
preprocessor optimize the constant?
I've always assumed it could not but that might be wrong.
thanks,
grant
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/1] aacraid: big endian issues
I've always assumed that byte swapping of constants would be optimized where a
variable would not :-)
I have confirmed in assembler output of the compiler that constant merely
become byte reversed constants optimized or no in at least one architectural
case. I have *not* confirmed that a variable byte reversal requires processing
overhead, as one could possibly expect the compiler to instead optimize by byte
reversing the constant when comparing. However, I will guarantee you that if
optimization is turned off in the compiler that such an optimization will not
take place...
I do not think this turns into a readability issue in either case and view this
as a simple cosmetic coding precaution much like likely()/unlikely() offers
hints to the compiler on code intent...
Sincerely -- Mark Salyzyn
-Original Message-
From: Grant Grundler [mailto:[EMAIL PROTECTED]
Sent: Tuesday, January 08, 2008 4:17 PM
To: Salyzyn, Mark
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH 1/1] aacraid: big endian issues
On Jan 8, 2008 12:48 PM, Salyzyn, Mark
[EMAIL PROTECTED] wrote:
Big endian systems issues discovered in the aacraid driver.
...
--- a/drivers/scsi/aacraid/comminit.c 2008-01-08
15:32:28.329810853 -0500
+++ b/drivers/scsi/aacraid/comminit.c 2008-01-08
15:37:35.633163607 -0500
@@ -301,10 +301,10 @@
if ((!aac_adapter_sync_cmd(dev, GET_ADAPTER_PROPERTIES,
0, 0, 0, 0, 0, 0, status+0, status+1,
status+2, NULL, NULL))
(status[0] == 0x0001)) {
- if (status[1] AAC_OPT_NEW_COMM_64)
+ if (status[1] le32_to_cpu(AAC_OPT_NEW_COMM_64))
...
Why apply le32_to_cpu() to the constant instead of the variable?
On systems were le32_to_cpu() is doing something, can gcc or
preprocessor optimize the constant?
I've always assumed it could not but that might be wrong.
thanks,
grant
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] aacraid: big endian issues
On Tue, Jan 08, 2008 at 01:17:15PM -0800, Grant Grundler wrote: Why apply le32_to_cpu() to the constant instead of the variable? On systems were le32_to_cpu() is doing something, can gcc or preprocessor optimize the constant? I've always assumed it could not but that might be wrong. $ grep constant_p include/linux/byteorder/* include/linux/byteorder/swabb.h:(__builtin_constant_p((__u32)(x)) ? \ include/linux/byteorder/swabb.h:(__builtin_constant_p((__u32)(x)) ? \ include/linux/byteorder/swab.h:(__builtin_constant_p((__u16)(x)) ? \ include/linux/byteorder/swab.h:(__builtin_constant_p((__u32)(x)) ? \ include/linux/byteorder/swab.h:(__builtin_constant_p((__u64)(x)) ? \ -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] aacraid: big endian issues
On Jan 8, 2008 1:37 PM, Salyzyn, Mark [EMAIL PROTECTED] wrote:
I've always assumed that byte swapping of constants would be optimized where
a variable would not :-)
definitely...I understood that. It just seemed odd usage.
I have confirmed in assembler output of the compiler that constant
merely become byte reversed constants optimized or no in at least
one architectural case.
Thanks!
And apologies, I didn't mean to cause you additional work since I
was just curious.
I have *not* confirmed that a variable byte reversal requires processing
overhead, as one could possibly expect the compiler to instead optimize by
byte reversing the constant when comparing. However, I will guarantee you
that if optimization is turned off in the compiler that such an optimization
will not take place...
:)
I do not think this turns into a readability issue in either case
and view this as a simple cosmetic coding precaution much
like likely()/unlikely() offers hints to the compiler on code intent...
Agreed - it's not a readability issue.
thanks,
grant
Sincerely -- Mark Salyzyn
-Original Message-
From: Grant Grundler [mailto:[EMAIL PROTECTED]
Sent: Tuesday, January 08, 2008 4:17 PM
To: Salyzyn, Mark
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH 1/1] aacraid: big endian issues
On Jan 8, 2008 12:48 PM, Salyzyn, Mark
[EMAIL PROTECTED] wrote:
Big endian systems issues discovered in the aacraid driver.
...
--- a/drivers/scsi/aacraid/comminit.c 2008-01-08
15:32:28.329810853 -0500
+++ b/drivers/scsi/aacraid/comminit.c 2008-01-08
15:37:35.633163607 -0500
@@ -301,10 +301,10 @@
if ((!aac_adapter_sync_cmd(dev, GET_ADAPTER_PROPERTIES,
0, 0, 0, 0, 0, 0, status+0, status+1,
status+2, NULL, NULL))
(status[0] == 0x0001)) {
- if (status[1] AAC_OPT_NEW_COMM_64)
+ if (status[1] le32_to_cpu(AAC_OPT_NEW_COMM_64))
...
Why apply le32_to_cpu() to the constant instead of the variable?
On systems were le32_to_cpu() is doing something, can gcc or
preprocessor optimize the constant?
I've always assumed it could not but that might be wrong.
thanks,
grant
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
