[sdcc-devel] [ sdcc-Bugs-1479882 ] Strange regression - 2.5.0 #1020 (Mar 18 2006) to 2.5.6 # (A

[SourceForge.net <no...@so...>]

Bugs item #1479882, was opened at 2006-05-01 19:11
Message generated for change (Comment added) made by bernhardheld
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1479882&group_id=599

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: z80 port
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Nobody/Anonymous (nobody)
>Assigned to: Bernhard Held (bernhardheld)
Summary: Strange regression - 2.5.0 #1020 (Mar 18 2006) to 2.5.6 # (A

Initial Comment:
The attached file compiles with both sdcc versions. It
behaves differently, that is, it works OK with 2.5.0,
but it doesn't with 2.5.6.

I compile using
sdcc -mz80 -c --std-c99 Finit.c

Sorry, I was not able to narrow the problem down any
further so far.
Please fix this before the new release. I'd really like
to use the new version, since it generates smaller,
faster code, which uses less RAM. This bug makes the
new version unuseable for me.

Philipp


----------------------------------------------------------------------

>Comment By: Bernhard Held (bernhardheld)
Date: 2006-05-10 22:48

Message:
Logged In: YES 
user_id=203539

Thanks for the analysis.
> This seems to be a bit more complicated
Yes it is ;-) I just learned that the z80 port behaves so 
much different from the mcs51 port. I now see the bug but 
I'll need some time to analyze what's going on. 

----------------------------------------------------------------------

Comment By: Philipp Krause (spth)
Date: 2006-05-10 00:14

Message:
Logged In: YES 
user_id=564030

This seems to be a bit more complicated than I thought.
&state->enemies[i] is used three times as a function
argument inside the big loop.
The first two times (lines 137ff and 250ff) it is calculated as:
4 * (-1(ix)) + e
-1(ix) is the loop counter, e is assigned at the beginning
of the function only.
The last time it is calculated different:
In line 289f
(-5(ix), -4(ix)) is used.
This value has been calculated in line 250ff as
(-5(ix), -4(ix)) += e
the previous value comes from line 178ff:
(-5(ix), -4(ix)) = (-3(ix), -2(ix)) + 0x1e

Since (-3(ix), -2(ix)) is never written inside the big loop
that means that the value used in the last call doesn't
change within the loop, while the value used in the first
two calls does change depending on the loop counter variable i.

Philipp



----------------------------------------------------------------------

Comment By: Philipp Krause (spth)
Date: 2006-05-09 23:37

Message:
Logged In: YES 
user_id=564030

I now see the attached .asm file. It is identical to the one
generated at my system. I will have a close look at the .asm
file again (I'm no longer sure my last description of the
problem was correct).

----------------------------------------------------------------------

Comment By: Philipp Krause (spth)
Date: 2006-05-09 23:25

Message:
Logged In: YES 
user_id=564030

I just checked with  today's SDCC :
mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08
2.5.6 # (May  9 2006) (UNIX)
built from the source from snapshot May 9, 2006.

The bug is still there; I can't compare with your.asm file
since I only see .c files attached to this bug report.
Philipp



----------------------------------------------------------------------

Comment By: Bernhard Held (bernhardheld)
Date: 2006-05-09 22:49

Message:
Logged In: YES 
user_id=203539

I still can't reproduce your problem:
- my assembler output uses different registers; the bug 
you are describing isn't there.
- my sdcc is build #4155
- could you please run
sdcc -S -mz80 bug-1479882.c
with the latest SDCC and compare it with bug-147882.asm 
(both files are attached)?
- please specify if you use a snapshot or if you build 
your own sdcc

----------------------------------------------------------------------

Comment By: Philipp Krause (spth)
Date: 2006-05-06 13:21

Message:
Logged In: YES 
user_id=564030

I found out what exactly is the problem here: The
&state->enemies[i]
is not calculated correctly (at least in the call to
cvu_set_sprite()/cv_memtovmemcpy()):

It is calculated as base_pointer + 4 * i. Since the 4 * i is
needed multiple times inside the loop the compiler just
calculates it once, stores it to a register and uses it
multiple times.
With --noinduction register c is used to hold the 4 * i.
Without --noinduction register e is used.
With --noinduction this register is initialized to 4 * i on
each iteration of the loop.
Without --noinduction this register is never written to
(except push/pop stuff), but used as if it was initialized
to 4 * i. Thus the value used in places where
&state->enemies[i] should be used becomes undefined.

Philipp


----------------------------------------------------------------------

Comment By: Philipp Krause (spth)
Date: 2006-05-06 10:32

Message:
Logged In: YES 
user_id=564030

I suppose that somehow wrong arguments are passed to the
last function call:

cvu_set_sprite(SPRITES, 4 + i, &state->enemies[i]);

If I just add another

for(i = 0; i < 4; i++)
cvu_set_sprite(SPRITES, 4 + i, &state->enemies[i]);

at the end of the function the incorrect behaviour seems to
go away.

Philipp


----------------------------------------------------------------------

Comment By: Philipp Krause (spth)
Date: 2006-05-03 23:25

Message:
Logged In: YES 
user_id=564030

I simplified it further. The problem is in the big loop. Any
further simplification of that loop seems to make the
problem go away.

typedef unsigned int size_t;
typedef unsigned char bool;

#include <stdint.h>

typedef uint16_t cv_vmemp;

extern void cv_memtovmemcpy(cv_vmemp dest, const void * src,
size_t n);

struct cvu_sprite
{
        uint8_t y;
        uint8_t x;
        uint8_t name;
        uint8_t tag;
};

#define cvu_set_sprite(base, number, sprite)
cv_memtovmemcpy((base) + (number) * 0x4, (sprite), 4)

extern void cvu_set_sprite_x(struct cvu_sprite *sprite, int x);
extern int cvu_get_sprite_x(const struct cvu_sprite *sprite);
extern void cvu_set_sprite_y(struct cvu_sprite *sprite, int y);
extern int cvu_get_sprite_y(const struct cvu_sprite *sprite);

#define cvu_set_sprite_color(sprite, color) (sprite)->tag =
((sprite)->tag & 0x80) | (color);

struct FlightState
{
	bool dead;
	struct cvu_sprite bullets[2];
	signed char bullet_xspeeds[2];
	signed char bullet_yspeeds[2];
	struct cvu_sprite enemies[4];
	unsigned char rowcounter;
	unsigned char balloon_position[2];
	signed char enemy_xspeed;
	int aagun_position[2];
	bool aagun_active;
	unsigned char aagun_direction;
	unsigned char aagun_color;
	bool aagun_moving;
};

#define SPRITES 0x3c00

extern uint8_t FantasyRand(const uint8_t seed);

void FlightEnemies(struct FlightState *state)
{
	unsigned char i;
	for(i = 0; i < 4; i++)
		cvu_set_sprite_x(&state->enemies[i], 50);
	for(i = 0; i < 4; i++)
	{
		int y;
		y = cvu_get_sprite_y(&state->enemies[i]);
		if(y != 207)
			y += FantasyRand(state->balloon_position[0] ^
state->balloon_position[1]) % 3;
		cvu_set_sprite_y(&state->enemies[i], 50);
		cvu_set_sprite(SPRITES, 4 + i, &state->enemies[i]);	
	}	
}


----------------------------------------------------------------------

Comment By: Philipp Krause (spth)
Date: 2006-05-03 23:07

Message:
Logged In: YES 
user_id=564030

Slightly simplified file still showing the bug below.
The bug goes away when I put line
cvu_set_sprite(SPRITES, 4 + i, &state->enemies[i]);
in a seperate loop.
I goes away when I replace
y += FantasyRand(state->balloon_position[0] ^
state->balloon_position[1]) % 3;
by
y += 1;
too.

typedef unsigned int size_t;
typedef unsigned char bool;

#include <stdint.h>

typedef uint16_t cv_vmemp;

extern void cv_memtovmemcpy(cv_vmemp dest, const void * src,
size_t n);

struct cvu_sprite
{
        uint8_t y;
        uint8_t x;
        uint8_t name;
        uint8_t tag;
};

#define cvu_set_sprite(base, number, sprite)
cv_memtovmemcpy((base) + (number) * 0x4, (sprite), 4)

extern void cvu_set_sprite_x(struct cvu_sprite *sprite, int x);
extern int cvu_get_sprite_x(const struct cvu_sprite *sprite);
extern void cvu_set_sprite_y(struct cvu_sprite *sprite, int y);
extern int cvu_get_sprite_y(const struct cvu_sprite *sprite);

#define cvu_set_sprite_color(sprite, color) (sprite)->tag =
((sprite)->tag & 0x80) | (color);

struct FlightState
{
	bool dead;
	struct cvu_sprite bullets[2];
	signed char bullet_xspeeds[2];
	signed char bullet_yspeeds[2];
	struct cvu_sprite enemies[4];
	unsigned char rowcounter;
	unsigned char balloon_position[2];
	signed char enemy_xspeed;
	int aagun_position[2];
	bool aagun_active;
	unsigned char aagun_direction;
	unsigned char aagun_color;
	bool aagun_moving;
};

#define SPRITES 0x3c00

extern uint8_t FantasyRand(const uint8_t seed);

void FlightEnemies(struct FlightState *state)
{
	unsigned char i;
	static unsigned char animation;
	for(i = 0; i < 4; i++)
	{
		int x, y;
		x = cvu_get_sprite_x(&state->enemies[i]);
		y = cvu_get_sprite_y(&state->enemies[i]);
		x += state->enemy_xspeed;
		if(y != 207)
		{
			y += FantasyRand(state->balloon_position[0] ^
state->balloon_position[1]) % 3;
			if(y >= 2)
				y -= 1;
			if(y >= 164)
				y = 164;
		}
		else
		{
			y = 60;
			x = 255;
		}
		if(x < -32)
		{
			cvu_set_sprite_y(&state->enemies[i], 207);
			continue;
		}
		cvu_set_sprite_x(&state->enemies[i], x);
		cvu_set_sprite_y(&state->enemies[i], y);
		cvu_set_sprite(SPRITES, 4 + i, &state->enemies[i]);	
	}	
}


----------------------------------------------------------------------

Comment By: Bernhard Held (bernhardheld)
Date: 2006-05-03 22:43

Message:
Logged In: YES 
user_id=203539

I latetly changed the induction code and felt responsible 
for this bug report. Unfortunately I don't know nothing 
about z80 and it's sdcc port. I thoroughly analyzed the 
mcs51 code instead. But I found no functional difference 
between the mcs51 code with resp. without induction. I 
assume now that the bug is not in the icode optimizer but 
somewhere in the z80 port.

It will dramatically speed up bug fixing if you could
- point at the assembler line, which is buggy and/or
- (at least) provide a set of numbers in the struct 
FlightState, which exposes the bug.

----------------------------------------------------------------------

Comment By: Philipp Krause (spth)
Date: 2006-05-02 00:16

Message:
Logged In: YES 
user_id=564030

Please replace
#include <stddef.h>
by
typedef unsigned int size_t;
typedef unsigned char bool;
when testing with the file atatched to the oroginal bug
report as I had a custom stddef.h file, of which only those
two lines are relevant here.


----------------------------------------------------------------------

Comment By: Philipp Krause (spth)
Date: 2006-05-01 21:27

Message:
Logged In: YES 
user_id=564030

The problem goes away if the --noinduction option is used.


----------------------------------------------------------------------

Comment By: Philipp Krause (spth)
Date: 2006-05-01 19:23

Message:
Logged In: YES 
user_id=564030

Title line got shortened (and I forgot to login):
The sdcc version showing the problem is
SDCC :
mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08
2.5.6 # (Apr 30 2006) (UNIX)


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1479882&group_id=599