CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to CCS Technical Support

union is not working

 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
hayee



Joined: 05 Sep 2007
Posts: 252

View user's profile Send private message

union is not working
PostPosted: Thu May 12, 2016 5:54 am     Reply with quote

Hi
I am using CCS compiler version 4.124 and Pic Controller PIC24HJ128GP306.

I am using union function. Made a simple program for testing but working not as it has to be.

Code:

#include <24HJ128GP306.h>
#fuses HS,NOWDT,PR
#use delay(crystal=20M)
//!#use rs232(baud=9600, xmit=PIN_F3,rcv=PIN_F2)
#use rs232(baud=9600, xmit=PIN_F5,rcv=PIN_F4)


union input
{
   int8 bits_8[4];
   int16 bits_16[2];
   int32 bits_32;
}value;


void main()
{
   
//!   value.bits_32=0;
//!   value.bits_16[0]=0;
//!   value.bits_16[1]=0;
//!   value.bits_8[0]=0;
//!   value.bits_8[1]=0;
//!   value.bits_8[2]=0;
//!   value.bits_8[3]=0;
   
   value.bits_16[0]=232;
   value.bits_16[1]=11;
   
   printf("H=%lu ",value.bits_32);
   printf("H=%lu ",value.bits_16[1]);
   printf("L=%lu ",value.bits_16[0]);
   printf("0=%u ",value.bits_8[0]);
   printf("1=%u ",value.bits_8[1]);
   printf("2=%u ",value.bits_8[2]);
   printf("3=%u ",value.bits_8[3]);
   printf("\r\n");
   
   while(TRUE)
   {
     
   }
}


and checked the output on Hyper Terminal as follows
Quote:

H=721128 H=11 L=232 0=65512 1=0 2=11 3=0

value.bits_8[0] having a value of 65512. How?

What is wrong in my code?
Kindly tell me
Ttelmah



Joined: 11 Mar 2010
Posts: 19510

View user's profile Send private message

PostPosted: Thu May 12, 2016 8:07 am     Reply with quote

You need to give the union the packed attribute.

The sizes are not what you think. In PCD, if you declare an array of int8 values, by default, each will actually be stored as an int16, so they can be 'word' aligned. Remember an 'int' in PCD, is actually an int16. Accessing bytes on odd boundaries is much harder than on the older chips, especially using an array index.
%u will default to being able to print an int16 as well.
So your int8 array, is actually using 8 bytes, and this is then the size of the union.....

Code:

union __attribute__((packed)) input
{
   int8 bits_8[4];
   int16 bits_16[2];
   int32 bits_32;
}value;


This is one of the 'learning experiences' of dealing with byte data on these larger chips.

Be very careful as well. If you use a int8 *, and then try to access an int16 value using this, and use an odd value, this will cause an address error trap, since accessing an int16 value will always use a word access, and this cannot be done on an odd byte address.
jeremiah



Joined: 20 Jul 2010
Posts: 1348

View user's profile Send private message

PostPosted: Thu May 12, 2016 11:01 am     Reply with quote

PCD shouldn't have any trouble with the 8bit array. It only buffers 8bit values when they are not word aligned. But an even sized array of 8bits is word aligned so it should be fine. It looks more like a display bug. 232 is 0xE8 in hex. 65512 is 0xFFE8 in hex. Looks like it is displaying a 16bit value, but the actual stored value is 8bits.

Looking at the sym file confirms this:
Code:

W0      @SCRATCH
W0L     _RETURN_
W0      @delay_us1.P1
W0 -W1  @READ_ROM_MEMORY.P2
W0 -W1  @PRINTFU32_9600_5893_5892.P3
W0 -W1  @DIV3232B.P4
W0      @PRINTFU16_9600_5893_5892.P5
W0      @delay_ms1.P6
W1      @SCRATCH
W1      @WRITE_PACKED_MEMORY.P1
W1      @READ_PACKED_MEMORY.P2
W2      @READ_PACKED_MEMORY.P1
W2      @WRITE_PACKED_MEMORY.P2
W2      @READ_ROM_MEMORY.P3
W2 -W3  @DIV3232B.P4
W3      @READ_ROM_MEMORY.P1
W3      @WRITE_PACKED_MEMORY.P2
W3      @READ_PACKED_MEMORY.P3
W4      @PRINTFU32_9600_5893_5892.P1
W4      @PRINTFU16_9600_5893_5892.P2
800-803 value
4780-47FF Stack


Only 4 bytes allocated for value. The LST file also shows a MOV.B used for the index in question, so I would wager a printf sign extension bug.
Ttelmah



Joined: 11 Mar 2010
Posts: 19510

View user's profile Send private message

PostPosted: Thu May 12, 2016 2:22 pm     Reply with quote

%u prints an unsigned int16 by default, not an int8 (on PCD).
int8 on PCD, is by default _signed_.
So it is converting a signed value to it's 16bit equivalent, and then printing this as unsigned.....

The int8 declaration needs to be changed to unsigned int8.

So not a bug, but a type conversion result.
Ttelmah



Joined: 11 Mar 2010
Posts: 19510

View user's profile Send private message

PostPosted: Fri May 13, 2016 1:07 am     Reply with quote

Just to expand on this. 0xE8, as a signed int8, is -18. -18 (signed int8) converted to a signed int16, is stored as 0xFFE8. This is implicitly done by passing an int8 to the in16 print. 0xFFE8, printed as _unsigned_, is 65512. Exactly what is being seen......

There are two important things here.

First I would always use the packed attribute for an operation like this. In this case it doesn't matter, but if at any point in the union, there are 'non array' bytes, data will not be aligned as wanted. This is vital when setting up things like structures into a union. If (for instance), you declared a structure of two 8 bit integers, for the LSB/MSB, these would not work, unless packed. So for anything where stuff is being declared to align data for something else, it is worth defaulting to packed. Smile

Then remember that in PCD, the default integers are int16, and signed.

At one point I actually tried using "#typdef unsigned" to avoid this, but this causes problems with some of the CCS code, so doesn't solve the problem.
Instead load the 'stdint.h' file, and the code then becomes:

Code:

#include <24HJ128GP306.h>
#fuses HS,NOWDT,PR
#use delay(crystal=20M)
#use rs232(baud=9600, xmit=PIN_F5,rcv=PIN_F4)

#include <stdint.h>

union __attribute__((packed)) input
{
   uint8_t bits_8[4];
   uint16_t bits_16[2];
   uint32_t bits_32;
}value;


void main()
{
   
//!   value.bits_32=0;
//!   value.bits_16[0]=0;
//!   value.bits_16[1]=0;
//!   value.bits_8[0]=0;
//!   value.bits_8[1]=0;
//!   value.bits_8[2]=0;
//!   value.bits_8[3]=0;
   
   value.bits_16[0]=232;
   value.bits_16[1]=11;
   
   printf("H=%lu ",value.bits_32);
   printf("H=%lu ",value.bits_16[1]);
   printf("L=%lu ",value.bits_16[0]);
   printf("0=%u ",value.bits_8[0]);
   printf("1=%u ",value.bits_8[1]);
   printf("2=%u ",value.bits_8[2]);
   printf("3=%u ",value.bits_8[3]);
   printf("\r\n");
   
   while(TRUE)
   {
     
   }
}


This functions correctly on all the PIC processors (and in fact works on things like the 80486 as well - not using CCS).

Because the values are now all stored as unsigned integers, when 0xE8, is converted to int16, it becomes 0x00E8, and will then print correctly.
jeremiah



Joined: 20 Jul 2010
Posts: 1348

View user's profile Send private message

PostPosted: Fri May 13, 2016 5:25 am     Reply with quote

Ttelmah wrote:
If (for instance), you declared a structure of two 8 bit integers, for the LSB/MSB, these would not work, unless packed. So for anything where stuff is being declared to align data for something else, it is worth defaulting to packed. Smile


Sorry for the slight sidetrack here, but wanted to ask if you could show an example here. I was trying to understand the technical need for packed in a struct with two 8 bit integers. I know the general desire for packed but from a technical standpoint, unless I am misunderstanding what you are referencing, a structure with 2 8 bit ints will align correctly without packed.

Here is what I am talking about:

Test Code
Code:

#case
#include <24fj64ga004.h>

typedef struct{
   unsigned int8 b0;
   unsigned int8 b1;
} test_struct_t;

typedef union{
   unsigned int16 value;
   struct{
      unsigned int8 b0;
      unsigned int8 b1;
   };
} test_union_t;

void main()
{
   test_struct_t s1;
   test_union_t  u1;
   
   s1.b0 = 0x03;
   s1.b1 = 0x85;
   
   u1.value = 0x1234;
   u1.b0 = 0x67;
   u1.b1 = 0x01;
   
   while(TRUE);
}


SYM excerpt
Code:

630.6   C1OUT
630.7   C2OUT
802-803 main.s1
804-805 main.u1
2780-27FF Stack


This shows both variables with sizes of 2 bytes, which is what I would expect.

LST excerpt
Code:

....................    test_struct_t s1;
....................    test_union_t  u1;
....................     
....................    s1.b0 = 0x03;
020C:  MOV.B   #3,W0L
020E:  MOV.B   W0L,802
....................    s1.b1 = 0x85;
0210:  MOV.B   #85,W0L
0212:  MOV.B   W0L,803
....................     
....................    u1.value = 0x1234;
0214:  MOV     #1234,W4
0216:  MOV     W4,804
....................    u1.b0 = 0x67;
0218:  MOV.B   #67,W0L
021A:  MOV.B   W0L,804
....................    u1.b1 = 0x01;
021C:  MOV.B   #1,W0L
021E:  MOV.B   W0L,805


This shows that both b0 and b1 correctly call byte aligned instructions at the correct addresses.

Is this the same scenario you are discussing? If not, could you lay out a small example? Sorry about me being so dense about this. I am probably just reading something incorrectly.

EDIT: I am not saying you shouldn't use packed. I'm just trying to understand the mechanics correctly here.
Ttelmah



Joined: 11 Mar 2010
Posts: 19510

View user's profile Send private message

PostPosted: Fri May 13, 2016 9:31 am     Reply with quote

May well depend on compiler.....

I've had it in the past _not_ align the structures used in the CCS default USB code correctly, unless I declared them to be packed. Suspect CCS may have fixed it.
Having been 'once bitten', I default to using packed (no reason not to, and it _ensures_ that the alignment does not get upset by the compiler...). Smile
jeremiah



Joined: 20 Jul 2010
Posts: 1348

View user's profile Send private message

PostPosted: Fri May 13, 2016 11:20 am     Reply with quote

Ok, makes sense. I've never run into it before, but now I know what to look for. I originally avoided ((packed)) because I wanted to make sure I didn't screw up and access misaligned data (16bit value on an odd address) and the default behavior of the compiler was shrinking them down to 8bits anyways.

I'm surprised CCS didn't make use of more __attribute__ directives. They tend to go with precompiler directives on a lot of stuff other compilers use the __attribute__ on.
Ttelmah



Joined: 11 Mar 2010
Posts: 19510

View user's profile Send private message

PostPosted: Fri May 13, 2016 11:58 am     Reply with quote

What you have to be careful about is any pointers to 16bit or 32bit values. 8bit values are always byte accessed. Smile
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group