Hello,
i have a problem with a specific argument of a function when i use a variable.
I don't even see my traces that indicate i enter the function.
I have problem only for the second argument.
I guessed a mix of unsigned with signed in arithmetic operation but i'm not sure.
Thanks.
Ps : i use the last version of SDCC 3.5.0
Ps : i have solve the problem with using structure pointer as argument but i'd like to understand my mistake. ;)
Some examples :
DisplayObject(object->_objectId,
objectInRoom->_coord.x * SQUARE_WIDTH + OBJECT_ORIGIN_X, // Result is 13
objectInRoom->_coord.y * SQUARE_HEIGHT + OBJECT_ORIGIN_Y) // Result is 113;
-> Don't work
UINT x = objectInRoom->_coord.x * SQUARE_WIDTH + OBJECT_ORIGIN_X;
DisplayObject(object->_objectId,
x,
objectInRoom->_coord.y * SQUARE_HEIGHT + OBJECT_ORIGIN_Y);
-> Don't work
UINT x = (UINT)objectInRoom->_coord.x * (UINT)SQUARE_WIDTH ;
x += (UINT)OBJECT_ORIGIN_X;
DisplayObject(object->_objectId,
x,
objectInRoom->_coord.y * SQUARE_HEIGHT + OBJECT_ORIGIN_Y);
-> Don't work
DisplayObject(object->_objectId,
objectInRoom->_coord.x * SQUARE_WIDTH, // Result is 0
objectInRoom->_coord.y * SQUARE_HEIGHT + OBJECT_ORIGIN_Y);
-> Work and see my traces
DisplayObject(
object->_objectId,
13,
objectInRoom->_coord.y * SQUARE_HEIGHT + OBJECT_ORIGIN_Y);
-> Work and see my traces
UINT x = 13;
DisplayObject(
object->_objectId,
x,
objectInRoom->_coord.y * SQUARE_HEIGHT + OBJECT_ORIGIN_Y);
-> Work and see my traces
Extract of my code, the entrance point is DrawRoomContent.
#define SCENE_ORIGIN_X 12
#define SCENE_ORIGIN_Y 42
#define OBJECT_ORIGIN_X (SCENE_ORIGIN_X + 1)
#define OBJECT_ORIGIN_Y (SCENE_ORIGIN_Y + 11)
#define SQUARE_WIDTH 8
#define SQUARE_HEIGHT 32
typedef struct{
UCHAR x, y;
} SCoord
typedef struct{
...
SCoord _coord;
} SObjectInRoom;
typedef struct
{
UINT _objectId;
...
} SObject;
void DisplayObject(UCHAR pObjectId, UINT x, UINT y)
{
SObject* object = NULL;
u8* video;
MY_TRACES
video = cpct_getScreenPtr((u8*)_memVideo, x, y);
...
UCHAR* objects = GetObjectsBuffer() + (SQUARE_WIDTH * MASKED * SQUARE_HEIGHT * object->_type);
cpct_drawSpriteMasked(objects, video, SQUARE_WIDTH, SQUARE_HEIGHT);
...
}
void DrawRoomContent()
{
...
DisplayObject(object->_objectId, objectInRoom->_coord.x * SQUARE_WIDTH + OBJECT_ORIGIN_X,
objectInRoom->_coord.y * SQUARE_HEIGHT + OBJECT_ORIGIN_Y);
...
}
I can't see where you initialize the struct pointer 'object' to something different to NULL, in the function 'DisplayObject'.
And in the function 'DrawRoomContent', where is the declaration / initialization of the struct pointer 'object'?
It's a global? Or...?
If you use '->', you are accessing the members of a struct pointer, but if you are accessing a struct (not a struct pointer), you must use '.' instead,
I think you know this, but better to be sure :)
There is no apparent flaw in your code. However, I'd like to see it complete, not only some snippets.
When you mark a line with "//Result is 13", you mean that result is actually 13 or that it should be 13 but it is not?
Sometimes, SDCC has a hard time calculating complex parameters. I have seen it even crash when compiling. It might be a problem with the compiler (or not), but you should use the debugger to see it. With indirect traces this may not be analized deeply enough.
Hello and thanks for your answers.
Here my full cpctelera project. The problem is in
code.cQuote from: ronaldo on 09:43, 06 September 15
When you mark a line with "//Result is 13", you mean that result is actually 13 or that it should be 13 but it is not?
It's 13 and it's what i want, i can see the value with a printf
Quote from: ronaldo on 09:43, 06 September 15
It might be a problem with the compiler (or not), but you should use the debugger to see it. With indirect traces this may not be analized deeply enough.
I saw the --debug option and sdcdb and but it seems complicated to use ::)
I tried to reproduce a problem but couldn't using 3.5.4 (#9311). Some bugfixes have occurred since 3.5.0. We'd also need to see the compile line you're using since the code generated will depend on optimization level and other flags.
It could very well be a peephole optimizer issue so try compiling with "--no-peep" and see if that works.
btw: sdcc inlines a small number of common functions in string.h:
#define memcpy(dst, src, n) __builtin_memcpy(dst, src, n)
#define strcpy(dst, src) __builtin_strcpy(dst, src)
#define strncpy(dst, src, n) __builtin_strncpy(dst, src, n)
#define strchr(s, c) __builtin_strchr(s, c)
#define memset(dst, c, n) __builtin_memset(dst, c, n)
Those functions may be inlined with ldir if that would be shorter than a function call --- sdcc function calls are very expensive so that will be the case if most parameters are constants. Your C versions of these functions will most certainly be slower :)
Quote from: Alcoholics Anonymous on 05:13, 10 September 15
It could very well be a peephole optimizer issue so try compiling with "--no-peep" and see if that works.
I try it, and the problem is always here.
Now i think it's a SDCC problem rather a lack of knowledge of myself about SDCC and CPC programming.
Quote from: Alcoholics Anonymous on 05:13, 10 September 15
btw: sdcc inlines a small number of common functions in string.h:
#define memcpy(dst, src, n) __builtin_memcpy(dst, src, n)
#define strcpy(dst, src) __builtin_strcpy(dst, src)
#define strncpy(dst, src, n) __builtin_strncpy(dst, src, n)
#define strchr(s, c) __builtin_strchr(s, c)
#define memset(dst, c, n) __builtin_memset(dst, c, n)
Those functions may be inlined with ldir if that would be shorter than a function call --- sdcc function calls are very expensive so that will be the case if most parameters are constants. Your C versions of these functions will most certainly be slower :)
You are right, l'll use it.
Thanks for your help :)
The version you have attached to the post has this code:
x = objectInRoom->_coord.x * SQUARE_WIDTH + OBJECT_ORIGIN_X;
y = objectInRoom->_coord.y * SQUARE_HEIGHT + OBJECT_ORIGIN_Y;
DisplayObject(object->_objectId, x, y);
Is it working this way?
I'm trying to debug it, but I don't now what to do to make your game enter that piece of code. At least at the start, it does not enter there.
@Alcoholics Anonymous (http://www.cpcwiki.eu/forum/index.php?action=profile;u=1116): I found this in the generated code
0647 E1 [10] 1405 pop hl
0648 E5 [11] 1406 push hl
Should it be a peephole for eliminating this 2 instructions? Do they have any real utility?
Quote from: ronaldo on 21:22, 10 September 15
0647 E1 [10] 1405 pop hl
0648 E5 [11] 1406 push hl
Should it be a peephole for eliminating this 2 instructions? Do they have any real utility?
You're probably reading push/pop there... pop/push is still valid.
I think this may be a compiler issue too so I'll try to take a closer look as well.
Sorry, you are right. I was doing this very fast because I had to go and I missread this part. However, I also think it might be a compiler problem related to --fomit-frame-pointer. The piece of code from which I copied this part is full of pushes and pops. I assume this is code complexification due to not being authorized to use IX. That may ultimately cause the error when calculations are done directly in the parameters.
Up to know, I don't know what do I have to do in your game to make it enter the portion of code we are analyzing. It enters DisplayRoomContent function after execution, but it does not call DisplayObject. I assume that it I need to have an object in the screen for the function to be called, but I don't know how to do it.
So, right now, I have been looking at the generated code and it seems to be right. I tried these two cases.
Case 1:
void DrawRoomContent()
{
...
if (object != NULL)
{
x = objectInRoom->_coord.x * SQUARE_WIDTH + OBJECT_ORIGIN_X;
y = objectInRoom->_coord.y * SQUARE_HEIGHT + OBJECT_ORIGIN_Y;
DisplayObject(object->_objectId, x, y);
}
...
}
Case 2:
void DrawRoomContent()
{
...
if (object != NULL)
{
DisplayObject(object->_objectId,
objectInRoom->_coord.x * SQUARE_WIDTH + OBJECT_ORIGIN_X,
objectInRoom->_coord.y * SQUARE_HEIGHT + OBJECT_ORIGIN_Y );
}
...
}
In both cases, code generated looks good, seems equivalente, and doesn't seem to contain any flaw. I would need to trace it with real values to check it better. Here you are both generated codes side by side:
[attachimg=1]
I need to experiment a little more, but both codes seem to do the job.
Regarding push & pops, @Alcoholics Anonymous (http://www.cpcwiki.eu/forum/index.php?action=profile;u=1116), I think this part is more like what I was referring to previously:
0619 E5 [11] 1369 push hl
1370 ;src/code.c:368: if (objectInRoom->_objectId != NOTHING)
061A E1 [10] 1371 pop hl
061B E5 [11] 1372 push hl
This part is previous to both cases, but I think it could be summed up as push hl.
I found something pretty odd (at least for me), that I think could be a problem. This function starts with this code:
void DrawRoomContent()
{
UCHAR obj;
SObjectInRoom* objectInRoom;
SObject* object;
SRoom* room = GetActiveRoom();
//SPoint pt;
UINT x, y;
for (obj = 0; obj < MAX_OBJ_IN_ROOM; obj++)
{
objectInRoom = &room->_objectsInRoom[obj];
...
This code aims to get a pointer to a room (SRoom*) and then use it to retrieve its objects. However, something odd happens after the call to GetActiveRoom is executed:
1327 ;src/code.c:356: void DrawRoomContent()
1328 ; ---------------------------------
1329 ; Function DrawRoomContent
1330 ; ---------------------------------
05E6 1331 _DrawRoomContent::
05E6 F5 [11] 1332 push af
05E7 F5 [11] 1333 push af
05E8 F5 [11] 1334 push af
05E9 3B [ 6] 1335 dec sp
1336 ;src/code.c:361: SRoom* room = GetActiveRoom();
05EA CDr00r00 [17] 1337 call _GetActiveRoom
;; *******************************************************************************************************************
;; ************ HL holds a valid pointer to the room, but next lines overwrite it without previously saving it! *********
;; *******************************************************************************************************************
1338 ;src/code.c:365: for (obj = 0; obj < MAX_OBJ_IN_ROOM; obj++)
05ED 7D [ 4] 1339 ld a,l
05EE 21 05 00 [10] 1340 ld hl,#5
05F1 39 [11] 1341 add hl,sp
05F2 C6 09 [ 7] 1342 add a, #0x09
05F4 77 [ 7] 1343 ld (hl),a
05F5 7C [ 4] 1344 ld a,h
05F6 CE 00 [ 7] 1345 adc a, #0x00
05F8 23 [ 6] 1346 inc hl
05F9 77 [ 7] 1347 ld (hl),a
05FA FD 21 02 00 [14] 1348 ld iy,#2
05FE FD 39 [15] 1349 add iy,sp
0600 FD 36 00 00 [19] 1350 ld 0 (iy),#0x00
0604 1351 00106$:
1352 ;src/code.c:367: objectInRoom = &room->_objectsInRoom[obj];
0604 21 02 00 [10] 1353 ld hl, #2+0
0607 39 [11] 1354 add hl, sp
0608 7E [ 7] 1355 ld a, (hl)
0609 87 [ 4] 1356 add a, a
060A 87 [ 4] 1357 add a, a
060B 5F [ 4] 1358 ld e,a
060C 21 05 00 [10] 1359 ld hl, #5
060F 39 [11] 1360 add hl, sp
0610 7E [ 7] 1361 ld a, (hl)
0611 23 [ 6] 1362 inc hl
0612 66 [ 7] 1363 ld h, (hl)
0613 6F [ 4] 1364 ld l, a
0614 16 00 [ 7] 1365 ld d,#0x00
0616 19 [11] 1366 add hl, de
0617 33 [ 6] 1367 inc sp
0618 33 [ 6] 1368 inc sp
0619 E5 [11] 1369 push hl
As you can see here, there is a "call _GetActiveRoom" which I have traced and correctly returns a pointer to the room in HL. However, just after calling the function, HL is immediately destroyed, losing the pointer to the room, which hasn't been stored anywhere. In fact, when code arrives to the point of using room pointer, it tries to get it from SP+5 (where room variable should be stored). Of course, the value it gets from there has nothing to do with the previously lost value from HL.
I have compiled the code with different options (--no-peep, --fomit-frame-pointer, nothing...) and the result is always the same. I have also tried to reproduce this behaviour creating a program with similar structure, but I cannot reproduce it. With a similar program, I get something like this:
82 ;src/main.c:42: void main(void) {
83 ; ---------------------------------
84 ; Function main
85 ; ---------------------------------
0023 86 _main::
0023 DD E5 [15] 87 push ix
0025 DD 21 00 00 [14] 88 ld ix,#0
0029 DD 39 [15] 89 add ix,sp
002B 21 F9 FF [10] 90 ld hl,#-7
002E 39 [11] 91 add hl,sp
002F F9 [ 6] 92 ld sp,hl
93 ;src/main.c:44: MyStruct *st = returnStructPointer();
0030 CDr1Fr00 [17] 94 call _returnStructPointer
0033 EB [ 4] 95 ex de,hl
;; ************************************************************************************************
;; ******* This time, HL is saved into DE before overwriting its contents, as expected *********
;; ************************************************************************************************
96 ;src/main.c:47: for (i=0; i < 2; i++) {
0034 DD 36 F9 00 [19] 97 ld -7 (ix),#0x00
0038 98 00105$:
99 ;src/main.c:49: printf("(%d, %d, %d)\n\r", a->id, a->x, a->y);
0038 21 02 00 [10] 100 ld hl,#0x0002
003B 19 [11] 101 add hl,de
003C DD 75 FA [19] 102 ld -6 (ix),l
003F DD 74 FB [19] 103 ld -5 (ix),h
As you see here, just after calling returnStructPointer function, the value is saved to DE (with ex de, hl) before overwritting HL. Then, when the pointer is required again, it is used from de (add hl, de). This works fine. I don't see why this behaviour difference.
@Alcoholics Anonymous (http://www.cpcwiki.eu/forum/profile/?u=1116), do you think this looks like a bug? I'm I missing something?
Quote from: ronaldo on 18:10, 11 September 15
It enters DisplayRoomContent function after execution, but it does not call DisplayObject. I assume that it I need to have an object in the screen for the function to be called, but I don't know how to do it.
I think it's exactly the problem, the function is not called with these argument i can deduct that because my printf are not displayed.
If i use the "good" parameters, the function is called and my printf are visibled.
It seems that something is writing into the sp area and destroying that local var.
Maybe the dec sp instruction and a sys. interrupt?
I had random problems with that in my compiler.
Quote from: ronaldo on 18:10, 11 September 15
In both cases, code generated looks good, seems equivalente, and doesn't seem to contain any flaw. I would need to trace it with real values to check it better. Here you are both generated codes side by side:
(http://www.cpcwiki.eu/forum/index.php?action=dlattach;topic=11002.0;attach=15951;image)
I don't understand why Comments are taking up valuable memory space. Assembly would never be taking up a byte of memory. If it's a start of a new component, it could still be where the comment is.
Where is the assembler code that takes room in the SP area for the variables x and y?
UCHAR obj;
SObjectInRoom* objectInRoom;
SObject* object;
SRoom* room = GetActiveRoom();
//SPoint pt;
UINT x, y; <-----------THIS
I mean, after calling GetActiveRoom(), I can't see the code that reserves 4 bytes for x and y:
05E6 1331 _DrawRoomContent::
05E6 F5 [11] 1332 push af
05E7 F5 [11] 1333 push af
05E8 F5 [11] 1334 push af
05E9 3B [ 6] 1335 dec sp
1336 ;src/code.c:361: SRoom* room = GetActiveRoom();
05EA CDr00r00 [17] 1337 call _GetActiveRoom
;; *******************************************************************************************************************
;; ************ HL holds a valid pointer to the room, but next lines overwrite it without previously saving it! *********
;; *******************************************************************************************************************
------------------------->MISSING????<----------------
1338 ;src/code.c:365: for (obj = 0; obj < MAX_OBJ_IN_ROOM; obj++)
05ED 7D [ 4] 1339 ld a,l
Quote from: FloppySoftware on 23:34, 11 September 15
Where is the assembler code that takes room in the SP area for the variables x and y?
I mean, after calling GetActiveRoom(), I can't see the code that reserves 4 bytes for x and y:
It's hard to say without seeing all the code. sdcc might not bother to create those variables if they are not used or since they are not volatile and are local, they may be kept in registers during their lifetime.
Quote
Regarding push & pops, @Alcoholics Anonymous (http://www.cpcwiki.eu/forum/index.php?action=profile;u=1116), I think this part is more like what I was referring to previously:
0619 E5 [11] 1369 push hl
1370 ;src/code.c:368: if (objectInRoom->_objectId != NOTHING)
061A E1 [10] 1371 pop hl
061B E5 [11] 1372 push hl
This part is previous to both cases, but I think it could be summed up as push hl.
Yes that combination can be removed. I have such a rule in the z88dk rulesets :)
Quote from: ronaldo on 19:43, 11 September 15
@Alcoholics Anonymous (http://www.cpcwiki.eu/forum/profile/?u=1116), do you think this looks like a bug? I'm I missing something?
Yes that could be a bug. What I am wondering about is if that is valid C.
void DrawRoomContent()
{
UCHAR obj;
SObjectInRoom* objectInRoom;
SObject* object;
SRoom* room = GetActiveRoom();
//SPoint pt;
UINT x, y;
...
I'm not sure you can initialize with anything but a constant. Does it work if you do it 'properly'?
void DrawRoomContent()
{
UCHAR obj;
SObjectInRoom* objectInRoom;
SObject* object;
SRoom* room;
//SPoint pt;
UINT x, y;
room = GetActiveRoom();
...
The omit-frame-pointer and especially old-ralloc stuff is not supported by sdcc anymore. There are bugs there that will probably never be fixed. Maybe if a code snippet can be made to reproduce the omit-frame-pointer problem they'll accept it. It's better to change the library code to save ix so that omit-frame-pointer doesn't have to be used.
Quote from: Alcoholics Anonymous on 02:06, 12 September 15
I'm not sure you can initialize with anything but a constant. Does it work if you do it 'properly'?
I tested it already. Same behaviour.
Quote from: Alcoholics Anonymous on 02:06, 12 September 15
The omit-frame-pointer and especially old-ralloc stuff is not supported by sdcc anymore. There are bugs there that will probably never be fixed. Maybe if a code snippet can be made to reproduce the omit-frame-pointer problem they'll accept it. It's better to change the library code to save ix so that omit-frame-pointer doesn't have to be used.
The thing is that this happens with and without modifiers. Compiling without --fomit-frame-pointer yields same result in the part where HL is overwritten.
However, I have created other simple programs trying to reproduce this problem and I can't. Don't understand why does it happen here.
Quote from: ronaldo on 08:46, 12 September 15
I tested it already. Same behaviour.
The thing is that this happens with and without modifiers. Compiling without --fomit-frame-pointer yields same result in the part where HL is overwritten.
I only get the problem when compiling with "--fomit-frame-pointer". Without it I get this (sdcc peephole off, max-allocs-per-node 2000):
call _GetActiveRoom
ld a,l
add a, #0x09
ld c,a
ld a,h
adc a, #0x00
ld b,a
;
ld -3 (ix),#0x00
which is correct code placing the returned pointer plus 9 into bc and zeroing the loop counter.
If you add "--reserve-regs-iy" to "--fomit-frame-pointer" which maybe the OP can try you also get correct code (max-allocs-per-node 200000 here, sdcc peephole on):
call _GetActiveRoom
ld de, #0x0009
add hl, de
ld -2 (ix),l
ld -1 (ix),h
;
ld -5 (ix),#0x00
There's something wrong with sdcc's cost function when using iy so quite often the code is better if you tell it not to use it.
You are right, @Alcoholics Anonymous (http://www.cpcwiki.eu/forum/index.php?action=profile;u=1116). I didn't pay enough attention to the compiler results, or maybe I was not looking at the correct file.
I have compiled again only with --no-peep and with no compiler options at all, and I get the exact same code as you. Code is correct without "--fomit-frame-pointer". I have even tested the game and DrawRoomContent function works and draws objects as expected.
[attach=2]
I think this clearly settles it: it's a bug due to --fomit-frame-pointer. The only problem I have is how to reproduce it. Maybe we should send the file "as is" to SDCC developers. Would it be okay?
Quote from: ronaldo on 20:52, 14 September 15
I think this clearly settles it: it's a bug due to --fomit-frame-pointer. The only problem I have is how to reproduce it. Maybe we should send the file "as is" to SDCC developers. Would it be okay?
That file is too long... no one wants to wade through that amount of code :)
This snippet will also reproduce the problem. I think whittle this down and then submit it:
#define NULL 0
#define MAX_OBJ_IN_ROOM 10
#define NB_DIR 6
#define NOTHING 0
#define MAX_MONSTER_IN_ROOM 3
#define SCENE_ORIGIN_X 12
#define SCENE_ORIGIN_Y 42
#define OBJECT_ORIGIN_X (SCENE_ORIGIN_X + 1)
#define OBJECT_ORIGIN_Y (SCENE_ORIGIN_Y + 11)
#define SQUARE_WIDTH 8
#define SQUARE_HEIGHT 32
typedef struct
{
unsigned char x, y;
} SCoord;
typedef struct
{
unsigned int _objectId;
SCoord _coord;
} SObjectInRoom;
typedef struct
{
unsigned char _roomId;
unsigned char _attrib;
unsigned char _dir;
unsigned char _nextRoomId[NB_DIR];
SObjectInRoom _objectsInRoom[MAX_OBJ_IN_ROOM];
unsigned char _monstersId[MAX_MONSTER_IN_ROOM];
} SRoom;
typedef struct
{
int visible:1;
int takable:1;
int activable:1;
int usable:1;
int hidden:1;
int bit5:1;
int bit6:1;
int bit7:1;
} SObjAttrib;
typedef struct
{
unsigned int _objectId;
unsigned char _type;
SObjAttrib _attrib;
unsigned char _data;
unsigned char _containerId;
} SObject;
extern SRoom* GetActiveRoom(void);
extern SObject* GetObjectFromWorld(unsigned int pObjectId);
extern void DisplayObject(unsigned char pObjectId, unsigned int x, unsigned int y);
void DrawRoomContent()
{
unsigned char obj;
SObjectInRoom* objectInRoom;
SObject* object;
SRoom* room;
unsigned int x, y;
room = GetActiveRoom();
for (obj = 0; obj < MAX_OBJ_IN_ROOM; obj++)
{
objectInRoom = &room->_objectsInRoom[obj];
if (objectInRoom->_objectId != NOTHING)
{
object = GetObjectFromWorld(objectInRoom->_objectId);
if (object != NULL)
{
x = objectInRoom->_coord.x * SQUARE_WIDTH + OBJECT_ORIGIN_X;
y = objectInRoom->_coord.y * SQUARE_HEIGHT + OBJECT_ORIGIN_Y;
DisplayObject(object->_objectId, x, y);
}
}
}
}