SDCC bug?

Page 1/2
| 2

Par lintweaker

Champion (311)

Portrait de lintweaker

02-10-2020, 17:51

I am trying out some C + ASM combi code using SDCC, using some code from:
used C VDP routines
The following two routines give odd results, I am wondering if this is a SDCC bug?

void puthex8(uint8 v)
{
	puthex(2, (uint16) v);
}

//------------------------------------------------------------------------------
void puthex16(uint16 v)
{
	puthex(4, v);
}

when puthex8 is called with:

putdec8(flashid);

where flashid is of type unsigned char. With flashid = 19
It should print '13' but instead it prints:

1900000000000190000... repeated...

A quick gcc equivalent shows the code should work

#include 
#include 
#include 

void puthex(uint8_t nibbles, uint16_t v);
void puthex8(uint8_t v);
unsigned char hwid;

void main() {

	hwid=19;
	puthex8(hwid);
};

void puthex(uint8_t nibbles, uint16_t v)
{
       int i = (int)nibbles - 1;
       while (i >= 0) {
              uint16_t aux = (v >> (i << 2)) & 0x000F;
              uint8_t n = aux & 0x000F;
              if (n > 9)
                     printf("%c",('A' + (n - 10)));
              else
                     printf("%c", ('0' + n));
              i--;
       }
}

//------------------------------------------------------------------------------
void puthex8(uint8_t v)
{
       puthex(2, (uint16_t) v);
}

EDIT: the forum is ruining the code oO

!login ou Inscrivez-vous pour poster

Par geijoenr

Master (180)

Portrait de geijoenr

02-10-2020, 19:34

It seems SDCC is not handling properly the cast of the parameter and messing up the stack.

Maybe try doing the cast as a separate variable in side puthex8 and see what happens?

on the other hand, the variable definitions inside the loop... I would be suspicious of those as well in SDCC.

Par thegeps

Hero (571)

Portrait de thegeps

03-10-2020, 01:06

Is it a typo? You wrote

putdec8(flashid)

Instead of puthex8. If it isn't a typo, can it be tha cause of code malfunction?

Par lintweaker

Champion (311)

Portrait de lintweaker

03-10-2020, 08:09

thegeps wrote:

Is it a typo? You wrote

putdec8(flashid)

Instead of puthex8. If it isn't a typo, can it be tha cause of code malfunction?

That was a typo Smile

Par lintweaker

Champion (311)

Portrait de lintweaker

03-10-2020, 08:10

Right, I'll try a specific version for outputting a 8-bit hex and see how that goes. I'll try the original version in a smaller program to see what ASM code is produced.

Par DarkSchneider

Paladin (939)

Portrait de DarkSchneider

03-10-2020, 10:06

It is a good habit to use the same type, as they can be typedef of diferent types between platforms. Then define the hwid as uint8_t.
Also, try some explicit castings:

hwid = (uint8_t)19;
puthex8((uint8_t) hwid);

Par lintweaker

Champion (311)

Portrait de lintweaker

03-10-2020, 11:01

DarkSchneider wrote:

It is a good habit to use the same type, as they can be typedef of diferent types between platforms. Then define the hwid as uint8_t.
Also, try some explicit castings:

hwid = (uint8_t)19;
puthex8((uint8_t) hwid);

Thanks, I still have to figure out which of the 'modern' types are supported with SDCC.

Par lintweaker

Champion (311)

Portrait de lintweaker

03-10-2020, 11:06

Found the culprit, it's the signedness of int8 for 'i' not being honoured. When i becomes 255 (so should be regarded as -1) the loop should stop but because of the signedness bug it is used as being 255 so >= 0 and the loop continues (endlessly).

void puthex(uint8 nibbles, uint16 v)
{
	int8 i = (int8)nibbles - 1;
	while (i >= 0) {
		uint16 aux = (v >> (i << 2)) & 0x000F;
		uint8 n = aux & 0x000F;
		if (n > 9)
			vdp_putchar('A' + (n - 10));
		else
			vdp_putchar('0' + n);
		i--;
	}
}

using 'int' for i works around the issue as a quick fix but then is is probably using 16 bit numbers (still have to check to what types translate to how many bytes).

Par ToriHino

Hero (600)

Portrait de ToriHino

03-10-2020, 16:44

I always use the 'modern' types as defined in the stdint.h file for this:

typedef signed char             int8_t;
typedef short int               int16_t;
typedef long int                int32_t;

typedef unsigned char           uint8_t;
typedef unsigned short int      uint16_t;
typedef unsigned long int       uint32_t;

Where of course first (int8_t) is one byte, second is two bytes and third is 4 bytes. I also use stdbool.h for the standard bool type.

Par ducasp

Champion (383)

Portrait de ducasp

03-10-2020, 18:36

lintweaker wrote:

Found the culprit, it's the signedness of int8 for 'i' not being honoured. When i becomes 255 (so should be regarded as -1) the loop should stop but because of the signedness bug it is used as being 255 so >= 0 and the loop continues (endlessly).

void puthex(uint8 nibbles, uint16 v)
{
	int8 i = (int8)nibbles - 1;
	while (i >= 0) {
		uint16 aux = (v >> (i << 2)) & 0x000F;
		uint8 n = aux & 0x000F;
		if (n > 9)
			vdp_putchar('A' + (n - 10));
		else
			vdp_putchar('0' + n);
		i--;
	}
}

using 'int' for i works around the issue as a quick fix but then is is probably using 16 bit numbers (still have to check to what types translate to how many bytes).

You can simplify that a lot:

uint8 hi = nibbles / 16;
uint8 lo = nibbles & 0x0f;

hi > 9 ? hi+='A' : hi+='0';
lo > 9 ? lo+='A' : lo+='0';
vdp_putchar(hi);
vdp_putchar(lo);

Par ducasp

Champion (383)

Portrait de ducasp

03-10-2020, 21:34

ducasp wrote:
lintweaker wrote:

Found the culprit, it's the signedness of int8 for 'i' not being honoured. When i becomes 255 (so should be regarded as -1) the loop should stop but because of the signedness bug it is used as being 255 so >= 0 and the loop continues (endlessly).

void puthex(uint8 nibbles, uint16 v)
{
	int8 i = (int8)nibbles - 1;
	while (i >= 0) {
		uint16 aux = (v >> (i << 2)) & 0x000F;
		uint8 n = aux & 0x000F;
		if (n > 9)
			vdp_putchar('A' + (n - 10));
		else
			vdp_putchar('0' + n);
		i--;
	}
}

using 'int' for i works around the issue as a quick fix but then is is probably using 16 bit numbers (still have to check to what types translate to how many bytes).

You can simplify that a lot:

uint8 hi = nibbles / 16;
uint8 lo = nibbles & 0x0f;

hi > 9 ? hi+='A' : hi+='0';
lo > 9 ? lo+='A' : lo+='0';
vdp_putchar(hi);
vdp_putchar(lo);

Hmmmm did not understood what you wanted, now I do, you can change your code a little to avoid type casting:

void puthex(uint8 nibbles, uint16 v)
{	
uint16 aux;
uint8 n;
	do {
		--nibbles;
		aux = (v >> (nibbles << 2)) & 0x000F;
		n = aux & 0x000F;
		if (n > 9)
			vdp_putchar('A' + (n - 10));
		else
			vdp_putchar('0' + n);		
	}
	while (nibbles);
}

By the way, get used to not use post increment or post decrement when not needed, as there is increased usage of memory/registers/cpu when you do so (code need to keep the original value for that instance and only increment later). Using pre-increment or pre-decrement when you are not evaluating the value prior to the operation is faster and code will occupy less space. Wink

Page 1/2
| 2