Problem with GCC code generation (WinAVR 2009)

Go To Last Post
8 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi!

I use (with -O2 option) GCC from WinAVR 20090313 version 4.3.2.

		unsigned char *p;
		unsigned short pp;
		cli();
 34e:	f8 94       	cli
		p = RingPointer;
 350:	f2 01       	movw	r30, r4
		*p ++ = 0xFF;
 352:	d2 01       	movw	r26, r4
 354:	8f ef       	ldi	r24, 0xFF	; 255
 356:	8d 93       	st	X+, r24
		*p ++ = 0x04;
 358:	84 e0       	ldi	r24, 0x04	; 4
 35a:	81 83       	std	Z+1, r24	; 0x01
 35c:	fd 01       	movw	r30, r26
 35e:	31 96       	adiw	r30, 0x01	; 1
		*p ++ = from;
 360:	11 96       	adiw	r26, 0x01	; 1
 362:	2c 93       	st	X, r18
		*p ++ = to;
 364:	61 83       	std	Z+1, r22	; 0x01
 366:	32 96       	adiw	r30, 0x02	; 2
		pp = (unsigned short)p;
		if ( (pp >> 8) == ((RAM_END - 0x100) >> 8) ) p = ring_start;
		RingPointer = p;
 368:	2f 01       	movw	r4, r30
		sei();
 36a:	78 94       	sei
register unsigned char * RingPointer asm( "r4" );
unsigned char * volatile ring_start = (unsigned char*) RAM_START;

Where is

if ( (pp >> 8) == ((RAM_END - 0x100) >> 8) ) p = ring_start;

?!

Does it work in new version of GCC? What with register variables in newer version of GCC?

Ilya

ps: This is work:

if ( (pp & 0xFF00) == ((RAM_END - 0x100) & 0xFF00) ) p = ring_start;
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You haven't shown enough context - the code could easily have been moved to before or after the sequence you posted. Post more of the .lss and note that I'm going to edit your thread title.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi, clawson!

clawson wrote:
You haven't shown enough context - the code could easily have been moved to before or after the sequence you posted.

No, I checked it.
clawson wrote:
Post more of the .lss

Ok.

	WaitForReply = 0;
 304:	10 92 04 01 	sts	0x0104, r1
 308:	08 95       	ret

[skiped]

	{
		unsigned char *p;
		unsigned short pp;
		cli();
 34e:	f8 94       	cli
		p = RingPointer;
 350:	f2 01       	movw	r30, r4
		*p ++ = 0xFF;
 352:	d2 01       	movw	r26, r4
 354:	8f ef       	ldi	r24, 0xFF	; 255
 356:	8d 93       	st	X+, r24
		*p ++ = 0x04;
 358:	84 e0       	ldi	r24, 0x04	; 4
 35a:	81 83       	std	Z+1, r24	; 0x01
 35c:	fd 01       	movw	r30, r26
 35e:	31 96       	adiw	r30, 0x01	; 1
		*p ++ = from;
 360:	11 96       	adiw	r26, 0x01	; 1
 362:	2c 93       	st	X, r18
		*p ++ = to;
 364:	61 83       	std	Z+1, r22	; 0x01
 366:	32 96       	adiw	r30, 0x02	; 2
		pp = (unsigned short)p;
		if ( (pp >> 8) == ((RAM_END - 0x100) >> 8) ) p = ring_start;
		RingPointer = p;
 368:	2f 01       	movw	r4, r30
		sei();
 36a:	78 94       	sei
 36c:	cb cf       	rjmp	.-106    	; 0x304 
		}
#define RAM_START	0x1100
#define RAM_END		(RAM_START + 0x7FFF)
register unsigned char * RingPointer asm( "r4" );
unsigned char * volatile ring_start = (unsigned char*) RAM_START;
void ProcTel( unsigned char from, unsigned char to, unsigned char fc );

If RAM_END redefine as 'RAM_START + 0x8000' then it work fine.

And this is work too:

#if defined(__ASSEMBLER__)
#define RAM_START	0x1100
#define RAM_END		(RAM_START + 0x7FFF)
#else
#define RAM_START	0x1100u
#define RAM_END		(RAM_START + 0x7FFFu)
#endif

But I don't like double define same constants for asm and C...

clawson wrote:
and note that I'm going to edit your thread title.

Ok, no matter ;-)

Ilya

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

501-q wrote:
Where is
if ( (pp >> 8) == ((RAM_END - 0x100) >> 8) ) p = ring_start;

Make sure you don't compare unsigned against signed. Enable warnings and fix them.

Quote:
Does it work in new version of GCC? What with register variables in newer version of GCC?
Yes, you can use R2..R7 as global registers.

Compile every module with -ffixed2 -ffixed-3 ... including modules contributing to libraries or make otherwise sure that no module unintentionally uses one of your global regs.

Moreover, notice that GCC can optimize global registers: global registers cannot be volatile; volatile only applies to memory storage classes, not to register.

This can lead to problems if you share a global reg between different IRQ levels.

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

And this is work too:

if ( (pp >> 8) == ((RAM_END - 0x100u) >> 8) ) p = ring_start;

With this:

#define RAM_START	0x1100
#define RAM_END		(RAM_START + 0x7FFF)

Problem was with compare between signed and unsigned shorts. But I don't undestand GCC's logic in this case.

WBR, Ilya.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi, SprinterSB!

SprinterSB wrote:
501-q wrote:
Where is
if ( (pp >> 8) == ((RAM_END - 0x100) >> 8) ) p = ring_start;

Make sure you don't compare unsigned against signed. Enable warnings and fix them.

Yes, there is warning in this line:

pb.c:143: warning: integer overflow in expression

But this warning exist in both cases: when it work and when it isn't work.

To fix this warning I have to double define same constants (but I don't like it):

#if defined(__ASSEMBLER__)
#define RAM_START	0x1100
#define RAM_END		(RAM_START + 0x7FFF)
#else
#define RAM_START	0x1100u
#define RAM_END		(RAM_START + 0x7FFFu)
#endif

Ilya

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Isn't RAMEND defined in avr/io.h?

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

SprinterSB wrote:
Isn't RAMEND defined in avr/io.h?

I use RAM_END as end of external RAM. All of external ram I use as ring buffer (in this case).

(ring_start need too: I wait for some event and then save part of ring buffer, change ring_start and wait same event again).

Ilya