GCC4.4: Squash multiple warnings due to strict aliasing

PATCH New Marek Vasut 3 participants 9 messages Sep 26, 2011 Arm (32-bit)

lore

Share & tools

lore

Marek Vasut

author

2011-09-26 02:06 UTC #0 lore raw

PATCH 1/1 GCC4.4: Squash multiple warnings due to strict aliasing

Signed-off-by Marek Vasut <[email protected]>

---
 arch/arm/include/asm/io.h |   30 ++++++++++++++++++------------
 1 files changed, 18 insertions(+), 12 deletions(-)

arch/arm/include/asm/io.h +18 ~~-12~~

index 1fbc531..61f4987 100644
@@ -78,43 +78,49 @@ static inline phys_addr_t virt_to_phys(void * vaddr)
 extern inline void __raw_writesb(unsigned int addr, const void *data, int bytelen)
 {
 	uint8_t *buf = (uint8_t *)data;
-	while(bytelen--)
-		__arch_putb(*buf++, addr);
+	int i;
+	for (i = 0; i < bytelen; i++)
+		__arch_putb(buf[i], addr);
 }
 
 extern inline void __raw_writesw(unsigned int addr, const void *data, int wordlen)
 {
 	uint16_t *buf = (uint16_t *)data;
-	while(wordlen--)
-		__arch_putw(*buf++, addr);
+	int i;
+	for (i = 0; i < wordlen; i++)
+		__arch_putw(buf[i], addr);
 }
 
 extern inline void __raw_writesl(unsigned int addr, const void *data, int longlen)
 {
 	uint32_t *buf = (uint32_t *)data;
-	while(longlen--)
-		__arch_putl(*buf++, addr);
+	int i;
+	for (i = 0; i < longlen; i++)
+		__arch_putl(buf[i], addr);
 }
 
 extern inline void __raw_readsb(unsigned int addr, void *data, int bytelen)
 {
 	uint8_t *buf = (uint8_t *)data;
-	while(bytelen--)
-		*buf++ = __arch_getb(addr);
+	int i;
+	for (i = 0; i < bytelen; i++)
+		buf[i] = __arch_getb(addr);
 }
 
 extern inline void __raw_readsw(unsigned int addr, void *data, int wordlen)
 {
 	uint16_t *buf = (uint16_t *)data;
-	while(wordlen--)
-		*buf++ = __arch_getw(addr);
+	int i;
+	for (i = 0; i < wordlen; i++)
+		buf[i] = __arch_getw(addr);
 }
 
 extern inline void __raw_readsl(unsigned int addr, void *data, int longlen)
 {
 	uint32_t *buf = (uint32_t *)data;
-	while(longlen--)
-		*buf++ = __arch_getl(addr);
+	int i;
+	for (i = 0; i < longlen; i++)
+		buf[i] = __arch_getl(addr);
 }
 
 #define __raw_writeb(v,a)	__arch_putb(v,a)

Signature · 1 line

1.7.5.4

Nick Thompson

2011-09-26 09:26 UTC #1 lore raw

PATCH 1/1 GCC4.4: Squash multiple warnings due to strict aliasing

Quoted 10 lines · Marek Vasut

Signed-off-by Marek Vasut <[email protected]>
---
 arch/arm/include/asm/io.h |   30 ++++++++++++++++++------------
 1 files changed, 18 insertions(+), 12 deletions(-)
arch/arm/include/asm/io.h +0 -0
index 1fbc531..61f4987 100644

arch/arm/include/asm/io.h static inline phys_addr_t virt_to_phys(void * vaddr) · line 85

Quoted 10 lines

arch/arm/include/asm/io.h +3 -2

@@ -78,43 +78,49 @@ static inline phys_addr_t virt_to_phys(void * vaddr)
 extern inline void __raw_writesb(unsigned int addr, const void *data, int bytelen)
 {
 	uint8_t *buf = (uint8_t *)data;
-	while(bytelen--)
-		__arch_putb(*buf++, addr);
+	int i;
+	for (i = 0; i < bytelen; i++)
+		__arch_putb(buf[i], addr);
 }

This fixes the problem in these use cases, but leaves the door open.

Would it be better to change the __arch_putb macro into an extern inline function instead which would catch these and future cases?

Nick.

Marek Vasut

author

2011-09-26 09:32 UTC #2 lore raw

PATCH 1/1 GCC4.4: Squash multiple warnings due to strict aliasing

Quoted 24 lines · Nick Thompson

Quoted 23 lines · Marek Vasut

Signed-off-by Marek Vasut <[email protected]>

---

 arch/arm/include/asm/io.h |   30 ++++++++++++++++++------------
 1 files changed, 18 insertions(+), 12 deletions(-)

arch/arm/include/asm/io.h +3 -2

index 1fbc531..61f4987 100644
@@ -78,43 +78,49 @@ static inline phys_addr_t virt_to_phys(void * vaddr)
 
 extern inline void __raw_writesb(unsigned int addr, const void *data,
 int bytelen) {
 
 	uint8_t *buf = (uint8_t *)data;
 
-	while(bytelen--)
-		__arch_putb(*buf++, addr);
+	int i;
+	for (i = 0; i < bytelen; i++)
+		__arch_putb(buf[i], addr);

Quoted 6 lines

arch/arm/include/asm/io.h static inline phys_addr_t virt_to_phys(void * vaddr) · line 88
Quoted 1 line
arch/arm/include/asm/io.h +0 -0
 }
This fixes the problem in these use cases, but leaves the door open.
Would it be better to change the __arch_putb macro into an extern inline function instead which would catch these and future cases?

Yes, but you'll need to do that on a much larger scale. Is anyone up for doing it ?

Quoted 2 lines

Nick.

Nick Thompson

2011-09-26 09:44 UTC #3 lore raw

PATCH 1/1 GCC4.4: Squash multiple warnings due to strict aliasing

Quoted 25 lines · Marek Vasut

Quoted 24 lines · Nick Thompson

Quoted 23 lines · Marek Vasut

Signed-off-by Marek Vasut <[email protected]>

---

 arch/arm/include/asm/io.h |   30 ++++++++++++++++++------------
 1 files changed, 18 insertions(+), 12 deletions(-)

arch/arm/include/asm/io.h +3 -2

index 1fbc531..61f4987 100644
@@ -78,43 +78,49 @@ static inline phys_addr_t virt_to_phys(void * vaddr)
 
 extern inline void __raw_writesb(unsigned int addr, const void *data,
 int bytelen) {
 
 	uint8_t *buf = (uint8_t *)data;
 
-	while(bytelen--)
-		__arch_putb(*buf++, addr);
+	int i;
+	for (i = 0; i < bytelen; i++)
+		__arch_putb(buf[i], addr);

Quoted 8 lines

Quoted 5 lines
arch/arm/include/asm/io.h static inline phys_addr_t virt_to_phys(void * vaddr) · line 88
Quoted 1 line
arch/arm/include/asm/io.h +0 -0
 }
This fixes the problem in these use cases, but leaves the door open.
Would it be better to change the __arch_putb macro into an extern inline function instead which would catch these and future cases?
Yes, but you'll need to do that on a much larger scale. Is anyone up for doing it ?

I don't follow that. I found only three (identical) definitions in arm, sparc and sh. In those three cases __raw_writeb were also (identical) macro 'aliases' for __arch_putb.

I guess you are referring to the testing required for all the boards in those three arches, or even just arm, with changes to all the (get|set)(b|w|l) cases? Maybe I see your point now...

Nick.

Marek Vasut

author

2011-09-26 09:48 UTC #4 lore raw

PATCH 1/1 GCC4.4: Squash multiple warnings due to strict aliasing

Quoted 34 lines · Nick Thompson

Quoted 33 lines · Marek Vasut
Quoted 31 lines · Nick Thompson
Quoted 15 lines · Marek Vasut
Signed-off-by Marek Vasut <[email protected]>
---
 arch/arm/include/asm/io.h |   30 ++++++++++++++++++------------
 1 files changed, 18 insertions(+), 12 deletions(-)
arch/arm/include/asm/io.h +0 -0
index 1fbc531..61f4987 100644
@@ -78,43 +78,49 @@ static inline phys_addr_t virt_to_phys(void *
 vaddr)
 
 extern inline void __raw_writesb(unsigned int addr, const void *data,
 int bytelen) {
arch/arm/include/asm/io.h static inline phys_addr_t virt_to_phys(void * · line 89
Quoted 10 lines
arch/arm/include/asm/io.h +3 -2
 
 	uint8_t *buf = (uint8_t *)data;
 
-	while(bytelen--)
-		__arch_putb(*buf++, addr);
+	int i;
+	for (i = 0; i < bytelen; i++)
+		__arch_putb(buf[i], addr);
 
 }
This fixes the problem in these use cases, but leaves the door open.
Would it be better to change the __arch_putb macro into an extern inline function instead which would catch these and future cases?

Quoted 6 lines

Quoted 2 lines
Yes, but you'll need to do that on a much larger scale. Is anyone up for doing it ?
I don't follow that. I found only three (identical) definitions in arm, sparc and sh. In those three cases __raw_writeb were also (identical) macro 'aliases' for __arch_putb.

Oh if it's only three arches then that's fine.

Quoted 6 lines

I guess you are referring to the testing required for all the boards in those three arches, or even just arm, with changes to all the (get|set)(b|w|l) cases? Maybe I see your point now...
Nick.

Wolfgang Denk

2011-09-26 11:30 UTC #5 lore raw

PATCH 1/1 GCC4.4: Squash multiple warnings due to strict aliasing

In reply to Marek Vasut

Dear Marek Vasut,

Quoted 6 lines · In message <[email protected]> you wrote:

Quoted 3 lines
Would it be better to change the __arch_putb macro into an extern inline function instead which would catch these and future cases?
Yes, but you'll need to do that on a much larger scale. Is anyone up for doing it ?

What exactly are the advantages, and what the disadvantages?

code size / memory footprint?

Best regards,

Wolfgang Denk

Signature · 6 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
ATTENTION: Despite Any Other Listing of Product Contents Found  Here-
on, the Consumer is Advised That, in Actuality, This Product Consists
Of 99.9999999999% Empty Space.

Marek Vasut

author

2011-09-26 17:37 UTC #6 lore raw

PATCH 1/1 GCC4.4: Squash multiple warnings due to strict aliasing

Quoted 6 lines · Wolfgang Denk

Dear Marek Vasut,
Quoted 3 lines · In message <[email protected]> you wrote:
Quoted 2 lines
Would it be better to change the __arch_putb macro into an extern inline function instead which would catch these and future cases?

Quoted 6 lines

Quoted 2 lines
Yes, but you'll need to do that on a much larger scale. Is anyone up for doing it ?
What exactly are the advantages, and what the disadvantages?
code size / memory footprint?

I assume converting those to inline function will yield no memory usage growth at all.

Quoted 4 lines

Best regards,
Wolfgang Denk

Wolfgang Denk

2011-09-26 18:33 UTC #7 lore raw

PATCH 1/1 GCC4.4: Squash multiple warnings due to strict aliasing

Dear Marek Vasut,

Quoted 7 lines · In message <[email protected]> you wrote:

Quoted 4 lines
What exactly are the advantages, and what the disadvantages?
code size / memory footprint?
I assume converting those to inline function will yield no memory usage growth at all.

Please don't assume, measure it. Thanks.

Best regards,

Wolfgang Denk

Signature · 7 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
IMPORTANT NOTICE TO PURCHASERS: The Entire Physical Universe,  Inclu-
ding  This Product, May One Day Collapse Back into an Infinitesimally
Small Space. Should  Another  Universe  Subsequently  Re-emerge,  the
Existence of This Product in That Universe Cannot Be Guaranteed.

Marek Vasut

author

2011-09-26 18:35 UTC #8 lore raw

PATCH 1/1 GCC4.4: Squash multiple warnings due to strict aliasing

Quoted 11 lines · Wolfgang Denk

Dear Marek Vasut,
Quoted 6 lines · In message <[email protected]> you wrote:
Quoted 3 lines
What exactly are the advantages, and what the disadvantages?
code size / memory footprint?
I assume converting those to inline function will yield no memory usage growth at all.
Please don't assume, measure it. Thanks.

Will do ... though I'd prefer to have this patch and the other one separate.

Cheers

Quoted 4 lines

Best regards,
Wolfgang Denk