SDCC bug?

Page 1/2
| 2

By lintweaker

Champion (457)

lintweaker's picture

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 or register to post comments

By geijoenr

Champion (362)

geijoenr's picture

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.

By thegeps

Paragon (1175)

thegeps's picture

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?

By lintweaker

Champion (457)

lintweaker's picture

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

By lintweaker

Champion (457)

lintweaker's picture

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.

By DarkSchneider

Paladin (984)

DarkSchneider's picture

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);

By lintweaker

Champion (457)

lintweaker's picture

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.

By lintweaker

Champion (457)

lintweaker's picture

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).

By ToriHino

Paladin (826)

ToriHino's picture

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.

By ducasp

Paladin (677)

ducasp's picture

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);

By ducasp

Paladin (677)

ducasp's picture

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