Compiler Bug: incorrect code generated with -Os

Go To Last Post

19 posts / 0 new

Author

Message

donblake

Level: Posting Freak

Joined: Sat. Jan 3, 2004

Posts: 1554 View posts

Location: Binghamton, NY, USA

Posted by donblake: Tue. Mar 27, 2012 - 01:04 AM

I've spent many hours the last week or two debugging a problem which is apparently the result of incorrect code generated with optimization set to -Os. The total code is fairly large (13,134 bytes). I've tried to reduce it to a minimum size but everything I've tried which is smaller works. The code works with no optimization and when I set the toolchain flavour to WinAVR-20100101.

The code is built for an ATmega644.

Here's the Native Studio 6 (Version 6.0.1703 - beta) -Os generated code:

ptr = IINCHIP_READ( Sn_TX_WR0( s ) );
  27b8:	ed b7       	in	r30, 0x3d	; 61
  27ba:	fe b7       	in	r31, 0x3e	; 62
  27bc:	36 96       	adiw	r30, 0x06	; 6
  27be:	0f b6       	in	r0, 0x3f	; 63
  27c0:	f8 94       	cli
  27c2:	fe bf       	out	0x3e, r31	; 62
  27c4:	0f be       	out	0x3f, r0	; 63
  27c6:	ed bf       	out	0x3d, r30	; 61
  27c8:	c8 01       	movw	r24, r16
  27ca:	0e 94 62 0e 	call	0x1cc4	; 0x1cc4 
ptr = ( ( ptr & 0x00ff ) << 8 ) + IINCHIP_READ( Sn_TX_WR0( s ) + 1 );
  27ce:	20 e0       	ldi	r18, 0x00	; 0
  27d0:	e9 01       	movw	r28, r18
  27d2:	c6 01       	movw	r24, r12
  27d4:	0e 94 62 0e 	call	0x1cc4	; 0x1cc4 
  27d8:	c8 0f       	add	r28, r24
  27da:	d1 1d       	adc	r29, r1

And here's the WinAVR-20100101 -Os generated code:

ptr = IINCHIP_READ( Sn_TX_WR0( s ) );
  292e:	ed b7       	in	r30, 0x3d	; 61
  2930:	fe b7       	in	r31, 0x3e	; 62
  2932:	36 96       	adiw	r30, 0x06	; 6
  2934:	0f b6       	in	r0, 0x3f	; 63
  2936:	f8 94       	cli
  2938:	fe bf       	out	0x3e, r31	; 62
  293a:	0f be       	out	0x3f, r0	; 63
  293c:	ed bf       	out	0x3d, r30	; 61
  293e:	c6 01       	movw	r24, r12
  2940:	0e 94 58 0e 	call	0x1cb0	; 0x1cb0 
ptr = ( ( ptr & 0x00ff ) << 8 ) + IINCHIP_READ( Sn_TX_WR0( s ) + 1 );
  2944:	18 2f       	mov	r17, r24
  2946:	00 e0       	ldi	r16, 0x00	; 0
  2948:	c7 01       	movw	r24, r14
  294a:	0e 94 58 0e 	call	0x1cb0	; 0x1cb0 
  294e:	08 0f       	add	r16, r24
  2950:	11 1d       	adc	r17, r1

The code makes 2 calls to IINCHIP_READ (which returns a uint8_t in r24) and combines the results in ptr, a uint16_t.

Note that the WinAVR-20100101 generated code saves the result from the first IINCHIP_READ call in r17 (mov 17, r24). The Native generated code doesn't use the result from the first IINCHIP_READ call at all.

The WinAVR-20100101 result is 0xD240 and the Native result is 0x0040.

I'd like to make a bug report but like I mentioned, I can only create this with a significant amount of code. Any ideas? Atmel support - should I submit a bug report and provide all my code?

Don

Tags:

Tools, Atmel Studio (AVR-related), ATmega64, ATmega644

donblake

Level: Posting Freak

Joined: Sat. Jan 3, 2004

Posts: 1554 View posts

Location: Binghamton, NY, USA

Posted by donblake: Tue. Mar 27, 2012 - 02:37 PM

Here's a couple of additional observations. There are 7 places in the code (which, by the way, isn't mine) that use nearly identical constructs. That is:

  val0 = IINCHIP_READ( Sn_TX_FSR0( s ) );
  val0 = ( val0 << 8 ) + IINCHIP_READ( Sn_TX_FSR0( s ) + 1 );

For 4 of them, correct code is generated. The other three have incorrect code generated (the result of the first IINCHIP_READ call is discarded and the upper byte of the uint16_t is set to zero).

I tried changing the code from addition to logical or:

  ptr = IINCHIP_READ( Sn_TX_WR0( s ) );
#if 0
  ptr = ( ( ptr & 0x00ff ) << 8 ) + IINCHIP_READ( Sn_TX_WR0( s ) + 1 );
#else
  ptr = ( ( ptr & 0x00ff ) << 8 ) | IINCHIP_READ( Sn_TX_WR0( s ) + 1 );
#endif

In all cases, the Native Studio 6 compiler then generates the correct code.

Don

david.prentice

Level: 10k+ Postman

Joined: Sat. Feb 12, 2005

Posts: 30454 View posts

Location: Wormshill, England

Posted by david.prentice: Tue. Mar 27, 2012 - 03:14 PM

This definitely looks like a bug to me.

It is certainly legal C.
OTOH, you could say that any author who dreams up such constructs deserves it.

It is such a common operation that most people would use more straightforward code. IINCHIP_READ() looks like a macro, so it may not evaluate to a strictly typed expression. Most people would say:

  ptr  = IINCHIP_READ( Sn_TX_WR0( s ) ) << 8;    // hi
  ptr |= IINCHIP_READ( Sn_TX_WR0( s+1 ) ) & 255;    // lo

They might even use += in the second statement. Have you tried that? I bet it works fine.

Perhaps SprinterSB can shed some light on it.

David.

donblake

Level: Posting Freak

Joined: Sat. Jan 3, 2004

Posts: 1554 View posts

Location: Binghamton, NY, USA

Posted by donblake: Tue. Mar 27, 2012 - 04:06 PM

david.prentice wrote:

IINCHIP_READ() looks like a macro, so it may not evaluate to a strictly typed expression.

My first thought was that IINCHIP_READ was a macro too. It's not, it's really a function that returns a uint8_t. Sn_TX_WR0 is a macro which evaluates to a uint16_t at run time.

david.prentice wrote:

Most people would say:
  ptr  = IINCHIP_READ( Sn_TX_WR0( s ) ) << 8;    // hi
  ptr |= IINCHIP_READ( Sn_TX_WR0( s+1 ) ) & 255;    // lo
They might even use += in the second statement. Have you tried that? I bet it works fine.

Just tried it. The |= compiles correctly. The += does not!

Don

david.prentice

Level: 10k+ Postman

Joined: Sat. Feb 12, 2005

Posts: 30454 View posts

Location: Wormshill, England

Posted by david.prentice: Tue. Mar 27, 2012 - 05:09 PM

If += does not work, that is a serious bug.

David.

stevech

Level: Raving Lunatic

Joined: Tue. Dec 18, 2001

Posts: 6119 View posts

Posted by stevech: Wed. Mar 28, 2012 - 05:12 AM

"IINCHIP_READ" looks like adaptation of WizNet's code for their ethernet modules.
(I use them, though I don't use their code).

Here's the function from their 5100 chip code

uint8 IINCHIP_READ(uint16 addr)
{
	uint8 data;

...

  return(data);
}

donblake

Level: Posting Freak

Joined: Sat. Jan 3, 2004

Posts: 1554 View posts

Location: Binghamton, NY, USA

Posted by donblake: Wed. Mar 28, 2012 - 12:04 PM

stevech wrote:

"IINCHIP_READ" looks like adaptation of WizNet's code for their ethernet modules.
(I use them, though I don't use their code).

Yes, that's what it is. I've done a lot of clean up on the code and fixed some problems. For example, I'm using the standard AVR data types not the original typedef's (uint8_t instead of uint8).

BTW: I've been successful in creating a much smaller code sample that exhibits this problem. I'm going to create an official bug report if I can ever get a logon to Bugzilla.

Don

SprinterSB

Level: Raving Lunatic

Joined: Thu. Dec 21, 2006

Posts: 2107 View posts

Location: Saar-Lor-Lux

Posted by SprinterSB: Wed. Mar 28, 2012 - 10:39 PM

f'up https://www.avrfreaks.net/index.p...

avrfreaks does not support Opera. Profile inactive.

donblake

Level: Posting Freak

Joined: Sat. Jan 3, 2004

Posts: 1554 View posts

Location: Binghamton, NY, USA

Posted by donblake: Thu. Mar 29, 2012 - 12:07 PM

SprinterSB wrote:

f'up https://www.avrfreaks.net/index.p...

Yup - I think that's it.

Now, how do we get Atmel to fix it (or at least make sure they're aware of it)? I've request a Bugzilla signon but still have not received a response (see Reporting bugs for the beta release of Atmel Studio 6).

Don

stevech

Level: Raving Lunatic

Joined: Tue. Dec 18, 2001

Posts: 6119 View posts

#10

Posted by stevech: Fri. Mar 30, 2012 - 02:02 AM

Does Atmel/Studio 6 mean that Atmel willsustain the Atmel variants of GCC?

danv

Level: Hangaround

Joined: Tue. Mar 1, 2011

Posts: 224 View posts

Location: Atmel, Norway

#11

Posted by danv: Fri. Mar 30, 2012 - 06:10 AM

I've put this bug in the internal bugzilla with reference number AVRTC-476 . Let's see what the compiler guys say.

Dan

donblake

Level: Posting Freak

Joined: Sat. Jan 3, 2004

Posts: 1554 View posts

Location: Binghamton, NY, USA

#12

Posted by donblake: Thu. May 10, 2012 - 11:52 PM

My latest tests indicate that this problem has been fixed with Atmel Studio 6 production release (Version 6.0.1843) with AVR Toolchain 8 Bit (3.4.0.663 - GCC 4.6.2).

Don

mostafanfs

Level: Rookie

Joined: Fri. Sep 7, 2012

Posts: 32 View posts

#13

Posted by mostafanfs: Sat. Jul 6, 2013 - 03:15 PM

I have a question
How does this function work?

uint8 IINCHIP_READ(uint16 addr)
{
      return (*((uint8*)addr));

or this function?

void IINCHIP_WRITE(uint16 addr, uint8 data)
{
      (*((vuint8*)addr)) = data;

I dont get it. How ATMEGA128 is connected to W5300?
W5300 has a 8 bit Data bus and 10 bit Address bus. Where to connect them ?

JohanEkdahl

Level: 10k+ Postman

Joined: Wed. Mar 27, 2002

Posts: 26945 View posts

Location: Lund, Sweden

#14

Posted by JohanEkdahl: Sat. Jul 6, 2013 - 03:22 PM

Quote:

I have a question
How does this function work?

While superficially the context is the same, your question is not much related to the original subject of this thread. (It is also a fact that this thread has been dormant for 14 months..)

When you have a question that is not directly related to anything you find here at AVRfreaks then you should start your own new thread. Use the "New subject" button (not the "New reply" button) for that.

I'll ask a moderator to break off your question into it's own thread.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here.

No guarantees, but if we don't report problems they won't get much of a chance to be fixed! Details/discussions at link given just above.

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 101079 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

#15

Posted by clawson: Sat. Jul 6, 2013 - 03:24 PM

The first one takes a 16 bit number 0x0000 to 0xFFFF. It casts it to be interpreted as a pointer to unsigned 8 bit then reads the contents of the location. So if location 0xBABE in memory holds 0xC7 and you call IINCHIP_READ(0xBABE) it will return 0xC7.

The write function is very similar, IINCHIP_WRITE(0xFACE, 0x55) will write the 8 bit value 0x55 to location 0xFACE. By casting the 0xFACE vale to be a pointer to uint8_t then writing through that pointer.

[Johan, I'm not going to attempt splitting a thread while not using a real computer, I'll do it later if no else has by then]

mostafanfs

Level: Rookie

Joined: Fri. Sep 7, 2012

Posts: 32 View posts

#16

Posted by mostafanfs: Sat. Jul 6, 2013 - 07:30 PM

@JohanEkdahl
Your right. I was aware of this. Then I realized there isn't much topics around W5300 so it would be better if I ask here. It's better than nothing! Plus I saw that two functions being discussed here.

Quote:

The write function is very similar, IINCHIP_WRITE(0xFACE, 0x55) will write the 8 bit value 0x55 to location 0xFACE. By casting the 0xFACE vale to be a pointer to uint8_t then writing through that pointer.

@clawson
Thank you. In IINCHIP_WRITE function how come it uses Volatile (vuint8) not (uint8) like in IINCHIP_READ ?

Actually something that I haven't realized yet is how exactly W5300 communicates with Mega128?
In IINCHIP_READ or IINCHIP_WRITE functions you are reading from or writing to a memory. What does it have to do with W5300 chip? How does it program W5300 registers?
Are the W5300 registers mapped to AVR memory somehow or what??

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 101079 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

#17

Posted by clawson: Sat. Jul 6, 2013 - 08:19 PM

Quote:

how come it uses Volatile (vuint8) not (uint8)

To ensure that the compiler makes the access and doesn't simply discard the code.

JohanEkdahl

Level: 10k+ Postman

Joined: Wed. Mar 27, 2002

Posts: 26945 View posts

Location: Lund, Sweden

#18

Posted by JohanEkdahl: Sat. Jul 6, 2013 - 08:37 PM

Quote:

Then I realized there isn't much topics around W5300 so it would be better if I ask here. It's better than nothing!

The alternative wasn't "nothing". The alternative was a separate thread. With that you could e.g. have a title that attracted more people interested in your specific problem. E.g. "Wiznet 5300 connections and driver functions". As it stands now it hides behind the (for you) meaningless title "Compiler bug [...]".

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here.

No guarantees, but if we don't report problems they won't get much of a chance to be fixed! Details/discussions at link given just above.

donblake

Level: Posting Freak

Joined: Sat. Jan 3, 2004

Posts: 1554 View posts

Location: Binghamton, NY, USA

#19

Posted by donblake: Sat. Jul 6, 2013 - 09:12 PM

mostafanfs wrote:

I dont get it. How ATMEGA128 is connected to W5300?
W5300 has a 8 bit Data bus and 10 bit Address bus. Where to connect them ?

I started this thread more than a year ago. I'm using a WIZnet W5100 and an ATMega644. I'm using the W5100 Serial Peripheral Interface (SPI) not the 15-bit address/8-bit data interface.

Don

Jump To

Search form

Compiler Bug: incorrect code generated with -Os

Tags: