PBP Using too many instructions

[The Master]

Hi, Im trying to optimise some of my code because its not running fast enough. Im trying to control PWM seperately on 24 pins. I did some tests to see what was taking so long and it seems to be the if statements.

IF 1=vPWMPos THEN
ENDIF

'Where vPWMPos is 1 the above code takes 7 instructions
'Where vPWMPos is not 1 the above code takes 8 instructions


'I also found out that the compiler does some clever optimisation
IF 1=1 THEN
ENDIF

'The above code uses no instructions at all because the compiler realises 1 will always equal 1

So, 7-8 instructions just for an if statement seems like quite a lot. Can anyone explain to me why it takes so many and if there is a away to optimise it at all.

TMR0 overflows after every 256 instructions. My if statements alone use 192 instructions. Add that to the rest of the code and it goes over 256 and this is only half the code i want to put in it. Using the prescaler isnt really an option either because that causes flickering

[Darrel Taylor]

... Can anyone explain to me why it takes so many and if there is a away to optimise it at all. ...

The WHY is 2 fold.
1. There's a CLRWDT in there. That can be eliminated with the NO_CLRWDT define.

2. PBP doesn't optimize Page switching as well as it does BANK switching. So before it can jump past the code (if the condition is false), it sets BOTH page bits, whether it needs to or not. And in most cases, it doesn't need to.

But even if you eliminated both of those issues with an ASM macro, it would still take 4-5 instructions, for a total of 120 cycles. Better, but not Best.

To really make it faster ... use the BRANCH or BRANCHL command. Then instead of 24 IF statements, it jumps right to the section it needs to be in, according to the the vPWMPos variable.

Branch will take about 16 instructions, BranchL will take a few more.
<br>

[The Master]

Thats interesting. Idealy i would like to get it down as far as possible. In this case the multiple if statements are required. It checks each pin's PWM value against the current PWM position. If it matches then that pin gets turned on. In my actual code i dont compare against a constant (1). Sorry for the confusion.

How would i go about doing the ASM macro thing?

Is there any other way to turn a pin on when a certain condition is met? I think this would probably work "pin = (pinValue=PWMPos)" but i wonder if that would get compiled to the same kind of code as an if statement.

Thinking about it, something like that would be an ideal solution. Whenever PWMPos overflows to 0 i have a seperate set of if statements that turn off any outputs whos values are less than 255. Both blocks of if statements are inside another if-then-else statement. If this method works then it should be able to do the whole thing with just 1 or 2 instructions

[Darrel Taylor]

... In this case the multiple if statements are required.

I have my doubts about that.
Multiple IF statements usually just means Spaghetti.

How would i go about doing the ASM macro thing?

Well, here's one way ...

;---------------------------------------------------------------------------
ASM
IfEqual  macro Bin, Cin, Lin          ; Bin is a BYTE variable
    RST?RP                            ; Cin is a constant to campare against
    movlw   Cin                       ; Lin is a label to jump to ...
    subwf   _vPWMPos,w                ;   if the condition is false
    btfsc   STATUS, Z
    goto    Lin
  endm
ENDASM
;-----[Here's how you use it]----------------------------------------------

@  IfEqual  _vPWMPos, 1, _L1
  ; code here executes if _vPWMPos = 1
L1:

@  IfEqual  _vPWMPos, 2, _L2
  ; code here executes if _vPWMPos = 2
L2:

In most cases, it will take 4-5 instructions.
But care must be taken to insure it doesn't cross a Page Boundary.

I still think you'll be better off using BRANCH.<hr>

I think this would probably work "pin = (pinValue=PWMPos)" but i wonder if that would get compiled to the same kind of code as an if statement.

That way only works if you can use the inverse of the result ...

myPIN = !(pinValue = vPWMPos)

Of course, the inverse of that is ...

myPIN = !(pinValue != vPWMPos)

[The Master]

Heres what my code looks like

IF vPWMPos=vPWM[0] THEN pA1=1
IF vPWMPos=vPWM[1] THEN pA2=1
IF vPWMPos=vPWM[2] THEN pA3=1
IF vPWMPos=vPWM[3] THEN pA4=1
IF vPWMPos=vPWM[4] THEN pA5=1
IF vPWMPos=vPWM[5] THEN pA6=1

Multiple ifs are needed because its possible for all of the vPWM values to be the same.

That ASM looks complicated but ill try and figure it out.

Im not at the PC i program on at the moment. Do you know if setting the value directly would work and if it does how many instructions it would take?

[Darrel Taylor]

That code is completely different from what you originally asked about.
Now you've got Array's involved.

When you get back to the PC, post what you have.
I'll take another look at the "Real" question.

But I still smell spaghetti.
<br>

[The Master]

The code i posted originally is what ive been using to test how many instruction cycles things take. I have 4 sockets (A to D) on this board each with 6 outputs (1 to 6).

'Define output pins
pA1 VAR PORTB.1
pA2 VAR PORTB.2
pA3 VAR PORTB.0
'etc...


'Then further down inside the TMR0 interrupt

'Is the vPWMPos equal to 0?
IF vPWMPos=0 THEN
	
	'Turn off any outputs that are less than 255
	IF vPWM[0]<255 THEN pA1=0
	IF vPWM[1]<255 THEN pA2=0
	IF vPWM[2]<255 THEN pA3=0
	'etc...
ELSE

	'If vPWMPos is less than any of the chanels then turn those chanels on
	IF vPWMPos=vPWM[0] THEN pA1=1
	IF vPWMPos=vPWM[1] THEN pA2=1
	IF vPWMPos=vPWM[2] THEN pA3=1
	'etc...
ENDIF

'Decrement the PWM counter
vPWMPos=vPWMPos-1

There doesnt have to be an array involved. I just used that to make other parts of the code a little easier. I could create a bunch of variables named vPWM0, vPWM1 etc instead. The important part is reducing the number of instructions an if statement uses or not using an if statement at all.

[Mike, K8LH]

Soft' PWM for that many outputs is difficult but you might be able to reduce 'overhead' using one of the following assembler methods;

Good luck on your project Sir. Regards, Mike

;
;  6 bit (single port), 256 step, 20 cycles (isochronous) for 16F
;
        movf    led+5,W         ; led[5] duty cycle, 0..255       |B0
        subwf   dcy,W           ; C = led[5] >= dcy               |B0
        rlf     shadow,F        ;                                 |B0
        movf    led+4,W         ; led[4] duty cycle, 0..255       |B0
        subwf   dcy,W           ; C = led[4] >= dcy               |B0
        rlf     shadow,F        ;                                 |B0
        movf    led+3,W         ; led[3] duty cycle, 0..255       |B0
        subwf   dcy,W           ; C = led[3] >= dcy               |B0
        rlf     shadow,F        ;                                 |B0
        movf    led+2,W         ; led[2] duty cycle, 0..255       |B0
        subwf   dcy,W           ; C = led[2] >= dcy               |B0
        rrf     shadow,F        ;                                 |B0
        movf    led+1,W         ; led[1] duty cycle, 0..255       |B0
        subwf   dcy,W           ; C = led[1] >= dcy               |B0
        rlf     shadow,F        ;                                 |B0
        movf    led+0,W         ; led[0] duty cycle, 0..255       |B0
        subwf   dcy,W           ; C = led[0] >= dcy               |B0
        rlf     shadow,F        ;                                 |B0
        xorlw   0xFF            ; for active hi outputs           |B0
        movwf   PORTB           ; update LEDs                     |B0
;
;  bump duty cycle counter
;
        movlw   b'00111111'     ; just in case                    |B0
        incf    dcy,F           ; bump duty cycle counter         |B0
        skpnz                   ; end-of-period? no, skip, else   |B0
        movwf   shadow          ; reset shadow                    |B0

;
;  6 bit (single port), 256 step, 15 cycles (isochronous) for 18F
;
        movf    dcy,W           ; duty cycle counter, 0..255      
        cpfsgt  Led+0           ; if(Led[0] >= dcy)               
        bcf     Shadow,0        ;   Shadow.0 = 0                  
        cpfsgt  Led+1           ; if(Led[1] >= dcy)               
        bcf     Shadow,1        ;   Shadow.1 = 0                  
        cpfsgt  Led+2           ;                                 
        bcf     Shadow,2        ;                                 
        cpfsgt  Led+3           ;                                 
        bcf     Shadow,3        ;                                 
        cpfsgt  Led+4           ;                                 
        bcf     Shadow,4        ;                                 
        cpfsgt  Led+5           ;                                 
        bcf     Shadow,5        ;                                 
        movff   Shadow,LATB     ;
;
;  bump duty cycle counter
;                         
        movlw   b'00111111'     ; just in case
        infsnz  dcy,F           ; if end-of-period                
        movwf   Shadow          ; reset shadow

[Jerson]

Very nice ASM coding

[The Master]

Ive been trying to get my head round this ASM code. Mike's ASM seems to do almost exactly what i want but i just want to clarify things as im completely new to ASM.

Is W some kind of temp byte? It appears that ASM compare commands only accept 1 input and that is always compared with W.

Ive read the part in the datasheet about CPFSGT. It says "Compare f with W, skip if f > W". I assume this means skip the following line of code

BCF looks pretty obvious. What is "Shadow" though? Is that a predefined variable of some kind or does it need declaring anywhere?

[Jerson]

Is W some kind of temp byte? It appears that ASM compare commands only accept 1 input and that is always compared with W.

Yes, W is the Accumulator on which almost all operations are performed.

What is "Shadow" though? Is that a predefined variable of some kind or does it need declaring anywhere?

Yes, shadow is a byte sized variable which holds a copy of the actual bits which turn on/off the leds attached to PORTB. It has to be declared like this

shadow:  var byte

[The Master]

So "LATB" is the ASM version of PORTB?

Is Shadow just a normal variable then? So i could call it LATBTemp if i wanted?

Is there a reason i would use Shadow instead of performing the operations directly on LATB?

[Art]

Heres what my code looks like

IF vPWMPos=vPWM[0] THEN pA1=1
IF vPWMPos=vPWM[1] THEN pA2=1
IF vPWMPos=vPWM[2] THEN pA3=1
IF vPWMPos=vPWM[3] THEN pA4=1
IF vPWMPos=vPWM[4] THEN pA5=1
IF vPWMPos=vPWM[5] THEN pA6=1

Multiple ifs are needed because its possible for all of the vPWM values to be the same.

Just looking at that, why not make the "pA" variables an array as well and just:

FOR  BLA = 1 to 6
IF vPWMPos=vPWM[BLA-1] THEN pA[BLA]=1
NEXT BLA

Otherwise I don't see how your claim that the array made things easier could be possible.

[The Master]

pA1 etc are references to output pins. In PBP you cant have an array that references anything else. Loops also take up extra instructions and in this case using less instructions is more important than writing neat code.

[Art]

Pity about a loop using more words. I wouldn't have thought so with so many IF statements.

I wonder if this would work to access ports as an array

vPWMPos var byte
vPWM var byte[6]
pA var portb.0
BLA var byte
'
FOR BLA = 0 TO 6
IF vPWMPos=vPWM[BLA] THEN pA[BLA]=1
NEXT BLA
'

In C, you can access arrays out of bounds like that, and we know portb pins are in consecutive memory locations.
. Don't know with PBP though.