gnupic: Thread: Re: [gnupic] Re: PIC18F47J53

I'm a little late but ...

Is the loop being opimized away?
When that happens, c will have no use.
Looking at the asm should show that.
The usual fix is volatile.

void sleep_a_while(byte c)
{
    volatile byte i,j;

    for(i=0; i<200; i++)
	for(j=0; j<c; j++) ;
}

    - Don

On Aug 13, Luis de Arquer propounded certain bytes, to wit:
> Subject: [gnupic] Re: PIC18F47J53
>
> I think I am giving up on this one.
> 
> I have put all the right SMD capacitors as close as I could, including
> a 10uF ceramic for vddcore, but still the same issue.
> 
> I am using pickit2 for this pic (18f47j53), and although I found a
> .dat file which supports it, I couldn't find anywhere in the microchip
> website saying it is supported really... so that could be the source
> of the grief.
> 
> Thank you all anyway :)
> 
> On Mon, Aug 12, 2013 at 10:54 PM, Bob Jacobsen ####@####.#### wrote:
> >
> > On Aug 12, 2013, at 1:09 PM, Luis de Arquer ####@####.#### wrote:
> >
> >> Problem number 1 is surely a C issue, which is the sleep function I made:
> >>
> >> ****************************
> >>
> >> typedef unsigned char byte;
> >>
> >> void sleep_a_while(byte c)              /* A */
> >> {
> >>  byte i,j;                                           /* B */
> >>
> >>  for(i=0; i<200; i++)
> >>    for(j=0; j<c; j++) ;
> >> }
> >>
> >>
> >> **************************
> >>
> >> Changing "byte" for "int" in any of those lines changes the timing. It
> >> makes the sleep time not only different -which makes sense since int
> >> is translated to int16 apparently, with its overhead-, but independent
> >> on c (the input parameter ! ).
> >
> >
> > Do you mean if you change (A) but not (B) or vice-versa?
> >
> > I'd check the assembler code to see what's happening to the upper byte of the int values.  If it's being left unchanged, all sorts of interesting things can happen, particularly if you call sleep-a-while with a value greater than 127.
> >
> > Bob
> > --
> > Bob Jacobsen, LBNL Physics Division
> > ####@####.#### +1-510-486-7355 AIM, Skype JacobsenRG
> >
> >
> >
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: ####@####.####
> > For additional commands, e-mail: ####@####.####
> >
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ####@####.####
> For additional commands, e-mail: ####@####.####
>-- End of excerpt from Luis de Arquer'

Bob,

I tried changing either B or both A and B together (didn't try A alone).

Don,

I think the code is not optimized. c seems to be used and everything
looks normal.

I haven't got the asm code here, I will upload it as soon as I get
home. In my opinion it looks OK. Considering the problem with the
clock I am having, possibly anything can happen really

Thanks

Luis



On Wed, Aug 14, 2013 at 1:45 AM, Don Wooton ####@####.#### wrote:
> I'm a little late but ...
>
> Is the loop being opimized away?
> When that happens, c will have no use.
> Looking at the asm should show that.
> The usual fix is volatile.
>
> void sleep_a_while(byte c)
> {
>     volatile byte i,j;
>
>     for(i=0; i<200; i++)
>         for(j=0; j<c; j++) ;
> }
>
>     - Don
>
> On Aug 13, Luis de Arquer propounded certain bytes, to wit:
>> Subject: [gnupic] Re: PIC18F47J53
>>
>> I think I am giving up on this one.
>>
>> I have put all the right SMD capacitors as close as I could, including
>> a 10uF ceramic for vddcore, but still the same issue.
>>
>> I am using pickit2 for this pic (18f47j53), and although I found a
>> .dat file which supports it, I couldn't find anywhere in the microchip
>> website saying it is supported really... so that could be the source
>> of the grief.
>>
>> Thank you all anyway :)
>>
>> On Mon, Aug 12, 2013 at 10:54 PM, Bob Jacobsen ####@####.#### wrote:
>> >
>> > On Aug 12, 2013, at 1:09 PM, Luis de Arquer ####@####.#### wrote:
>> >
>> >> Problem number 1 is surely a C issue, which is the sleep function I made:
>> >>
>> >> ****************************
>> >>
>> >> typedef unsigned char byte;
>> >>
>> >> void sleep_a_while(byte c)              /* A */
>> >> {
>> >>  byte i,j;                                           /* B */
>> >>
>> >>  for(i=0; i<200; i++)
>> >>    for(j=0; j<c; j++) ;
>> >> }
>> >>
>> >>
>> >> **************************
>> >>
>> >> Changing "byte" for "int" in any of those lines changes the timing. It
>> >> makes the sleep time not only different -which makes sense since int
>> >> is translated to int16 apparently, with its overhead-, but independent
>> >> on c (the input parameter ! ).
>> >
>> >
>> > Do you mean if you change (A) but not (B) or vice-versa?
>> >
>> > I'd check the assembler code to see what's happening to the upper byte of the int values.  If it's being left unchanged, all sorts of interesting things can happen, particularly if you call sleep-a-while with a value greater than 127.
>> >
>> > Bob
>> > --
>> > Bob Jacobsen, LBNL Physics Division
>> > ####@####.#### +1-510-486-7355 AIM, Skype JacobsenRG
>> >
>> >
>> >
>> >
>> >
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: ####@####.####
>> > For additional commands, e-mail: ####@####.####
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: ####@####.####
>> For additional commands, e-mail: ####@####.####
>>-- End of excerpt from Luis de Arquer'
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ####@####.####
> For additional commands, e-mail: ####@####.####
>

Gents,

In case someone is still interested, I am posting here the asm code
for the nested loop.
r0x0n are declared as registers with:

; Internal registers
.registers    udata_ovr    0x0000
r0x00    res    1
r0x01    res    1
r0x02    res    1
r0x03    res    1
r0x04    res    1
r0x05    res    1
r0x06    res    1

I was wondering something about this code here:

    MOVLW    0xc8
    SUBWF    r0x01, W

These instructions will substract W (0xc8) from r0x01. So it will
first make the 2's complement of W I guess. W contains 0xc8, which is
higher than 0x7F... does it imply that the carry bit will be set
whatever the result? In other words, W contains 200 decimal, but is
the PIC happy with representing -200 in a single byte?


Here all the code as promised:

In both cases, the function is called like

    MOVLW    0xd0                      ;whatever parameter
    MOVWF    POSTDEC1
    CALL    _sleep_a_while

*********************************************

First case -> works great:
--------------------------------

void sleep_a_while(byte c)
{
  byte i,j;

  for(i=0; i<200; i++)
    for(j=0; j<c; j++) ;
}

; ; Starting pCode block
S_test__sleep_a_while    code
_sleep_a_while:
;    .line    16; test.c    void sleep_a_while(byte c)
    MOVFF    FSR2L, POSTDEC1
    MOVFF    FSR1L, FSR2L
    MOVFF    r0x00, POSTDEC1
    MOVFF    r0x01, POSTDEC1
    MOVFF    r0x02, POSTDEC1
    MOVLW    0x02
    MOVFF    PLUSW2, r0x00
;    .line    20; test.c    for(i=0; i<200; i++)
    CLRF    r0x01
_00115_DS_:
;    .line    21; test.c    for(j=0; j<c; j++) ;
    CLRF    r0x02
_00108_DS_:
    MOVF    r0x00, W
    SUBWF    r0x02, W
    BC    _00111_DS_
    INCF    r0x02, F
    BRA    _00108_DS_
_00111_DS_:
;    .line    20; test.c    for(i=0; i<200; i++)
    INCF    r0x01, F
    MOVLW    0xc8
    SUBWF    r0x01, W
    BNC    _00115_DS_
    MOVFF    PREINC1, r0x02
    MOVFF    PREINC1, r0x01
    MOVFF    PREINC1, r0x00
    MOVFF    PREINC1, FSR2L
    RETURN

**********************************************************
**********************************************************

Second case: delay independent on input parameter c
---------------------------------------------------------------------

void sleep_a_while(byte c)
{
  int i,j;                      /* This is the only change! */

  for(i=0; i<200; i++)
    for(j=0; j<c; j++) ;
}

; ; Starting pCode block
S_test__sleep_a_while    code
_sleep_a_while:
;    .line    16; test.c    void sleep_a_while(byte c)
    MOVFF    FSR2L, POSTDEC1
    MOVFF    FSR1L, FSR2L
    MOVFF    r0x00, POSTDEC1
    MOVFF    r0x01, POSTDEC1
    MOVFF    r0x02, POSTDEC1
    MOVFF    r0x03, POSTDEC1
    MOVFF    r0x04, POSTDEC1
    MOVFF    r0x05, POSTDEC1
    MOVFF    r0x06, POSTDEC1
    MOVLW    0x02
    MOVFF    PLUSW2, r0x00
;    .line    20; test.c    for(i=0; i<200; i++)
    CLRF    r0x01
    CLRF    r0x02
_00115_DS_:
;    .line    21; test.c    for(j=0; j<c; j++) ;
    CLRF    r0x03
    CLRF    r0x04
_00108_DS_:
    MOVFF    r0x00, r0x05
    CLRF    r0x06
    MOVF    r0x04, W
    ADDLW    0x80
    MOVWF    PRODL
    MOVF    r0x06, W
    ADDLW    0x80
    SUBWF    PRODL, W
    BNZ    _00125_DS_
    MOVF    r0x05, W
    SUBWF    r0x03, W
_00125_DS_:
    BC    _00111_DS_
    INFSNZ    r0x03, F
    INCF    r0x04, F
    BRA    _00108_DS_
_00111_DS_:
;    .line    20; test.c    for(i=0; i<200; i++)
    INFSNZ    r0x01, F
    INCF    r0x02, F
    MOVF    r0x02, W
    ADDLW    0x80
    ADDLW    0x80
    BNZ    _00126_DS_
    MOVLW    0xc8
    SUBWF    r0x01, W
_00126_DS_:
    BNC    _00115_DS_
    MOVFF    PREINC1, r0x06
    MOVFF    PREINC1, r0x05
    MOVFF    PREINC1, r0x04
    MOVFF    PREINC1, r0x03
    MOVFF    PREINC1, r0x02
    MOVFF    PREINC1, r0x01
    MOVFF    PREINC1, r0x00
    MOVFF    PREINC1, FSR2L
    RETURN

On Wed, Aug 14, 2013 at 11:08 AM, Luis de Arquer ####@####.#### wrote:
> Bob,
>
> I tried changing either B or both A and B together (didn't try A alone).
>
> Don,
>
> I think the code is not optimized. c seems to be used and everything
> looks normal.
>
> I haven't got the asm code here, I will upload it as soon as I get
> home. In my opinion it looks OK. Considering the problem with the
> clock I am having, possibly anything can happen really
>
> Thanks
>
> Luis
>
>
>
> On Wed, Aug 14, 2013 at 1:45 AM, Don Wooton ####@####.#### wrote:
>> I'm a little late but ...
>>
>> Is the loop being opimized away?
>> When that happens, c will have no use.
>> Looking at the asm should show that.
>> The usual fix is volatile.
>>
>> void sleep_a_while(byte c)
>> {
>>     volatile byte i,j;
>>
>>     for(i=0; i<200; i++)
>>         for(j=0; j<c; j++) ;
>> }
>>
>>     - Don
>>
>> On Aug 13, Luis de Arquer propounded certain bytes, to wit:
>>> Subject: [gnupic] Re: PIC18F47J53
>>>
>>> I think I am giving up on this one.
>>>
>>> I have put all the right SMD capacitors as close as I could, including
>>> a 10uF ceramic for vddcore, but still the same issue.
>>>
>>> I am using pickit2 for this pic (18f47j53), and although I found a
>>> .dat file which supports it, I couldn't find anywhere in the microchip
>>> website saying it is supported really... so that could be the source
>>> of the grief.
>>>
>>> Thank you all anyway :)
>>>
>>> On Mon, Aug 12, 2013 at 10:54 PM, Bob Jacobsen ####@####.#### wrote:
>>> >
>>> > On Aug 12, 2013, at 1:09 PM, Luis de Arquer ####@####.#### wrote:
>>> >
>>> >> Problem number 1 is surely a C issue, which is the sleep function I made:
>>> >>
>>> >> ****************************
>>> >>
>>> >> typedef unsigned char byte;
>>> >>
>>> >> void sleep_a_while(byte c)              /* A */
>>> >> {
>>> >>  byte i,j;                                           /* B */
>>> >>
>>> >>  for(i=0; i<200; i++)
>>> >>    for(j=0; j<c; j++) ;
>>> >> }
>>> >>
>>> >>
>>> >> **************************
>>> >>
>>> >> Changing "byte" for "int" in any of those lines changes the timing. It
>>> >> makes the sleep time not only different -which makes sense since int
>>> >> is translated to int16 apparently, with its overhead-, but independent
>>> >> on c (the input parameter ! ).
>>> >
>>> >
>>> > Do you mean if you change (A) but not (B) or vice-versa?
>>> >
>>> > I'd check the assembler code to see what's happening to the upper byte of the int values.  If it's being left unchanged, all sorts of interesting things can happen, particularly if you call sleep-a-while with a value greater than 127.
>>> >
>>> > Bob
>>> > --
>>> > Bob Jacobsen, LBNL Physics Division
>>> > ####@####.#### +1-510-486-7355 AIM, Skype JacobsenRG
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > ---------------------------------------------------------------------
>>> > To unsubscribe, e-mail: ####@####.####
>>> > For additional commands, e-mail: ####@####.####
>>> >
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: ####@####.####
>>> For additional commands, e-mail: ####@####.####
>>>-- End of excerpt from Luis de Arquer'
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: ####@####.####
>> For additional commands, e-mail: ####@####.####
>>

On Aug 17, 2013, at 7:06 PM, Luis de Arquer ####@####.#### wrote:

> 
> First case -> works great:
> --------------------------------
> 
> void sleep_a_while(byte c)
> {
>  byte i,j;
> 
>  for(i=0; i<200; i++)
>    for(j=0; j<c; j++) ;
> }
> 
> 
> **********************************************************
> **********************************************************
> 
> Second case: delay independent on input parameter c
> ---------------------------------------------------------------------
> 
> void sleep_a_while(byte c)
> {
>  int i,j;                      /* This is the only change! */
> 
>  for(i=0; i<200; i++)
>    for(j=0; j<c; j++) ;
> }
> 


> In both cases, the function is called like
> 
>    MOVLW    0xd0                      ;whatever parameter
>    MOVWF    POSTDEC1
>    CALL    _sleep_a_while
> 


Decimal 208 is the integer 0x00D0.  In C, int is a signed type.

C does down-conversion by truncation.  If you then convert that 208 to a byte type, it's 0xD0, which is a _negative_ number.  

When j is an int, the comparison is signed, so c is sign-extended to an int and will immediately be less than j, hence fall through.

Moral:  If you want unsigned counters, tell C to use unsigned types.

Bob

--
Bob Jacobsen, LBNL Physics Division
####@####.#### +1-510-486-7355 AIM, Skype JacobsenRG

Bob,


On Sat, Aug 17, 2013 at 6:40 PM, Bob Jacobsen ####@####.#### wrote:
>
> Decimal 208 is the integer 0x00D0.  In C, int is a signed type.
>
> C does down-conversion by truncation.  If you then convert that 208 to a byte type, it's 0xD0, which is a _negative_ number.
>
> When j is an int, the comparison is signed, so c is sign-extended to an int and will immediately be less than j, hence fall through.
>
> Moral:  If you want unsigned counters, tell C to use unsigned types.
>


I think you are right in the moral, and it's a good point. It doesn't
look right to mix signed and unsigned counters.

However, in this case, it doesn't make a difference. Reason is I
defined 'byte' as:

typedef unsigned char byte;

When c is extended to int, it is not sign-extended. I have tried it on
gcc, which should (?) be the same:

#include <stdio.h>

typedef unsigned int byte;

int main()
{
  byte c = 0xD0;
  int a = (int) c;
  printf("Sign extending c gives 0x%08X\n", a);
  return 0;
}

********************
Output:

Sign extending c gives 0x000000D0

**************************



I haven't had enough time to make all the tests that I want, but it
has to be the XINST bit in the config words. I thought the only
difference it made was to add a few instructions to the instruction
set, but it also adds *indexed addressing mode*. So, every time I was
accessing ACCESS banked memory lower than 0x60 (as I bet are the
registers r0x0n), what it was actually doing is this (from pic
datasheet):


the file address of the
instruction is not interpreted as the lower byte of an
address (used with the BSR in Direct Addressing) or as
an 8-bit address in the Access Bank. Instead, the value
is interpreted as an offset value to an Address Pointer
specified by FSR2. The offset and the contents of
FSR2 are added to obtain the target address of the
operation.


Setting XINST=OFF seems to fix everything, including the "random
clock" issue I had.

Has anyone used extended instruction set before? Maybe it is not
supported in gputils? Could this be a bug in gplink, in sdcc, or in my
code?

Regards,

Luis