I don't know much about this pic18 code.
You can use
movlw samples_x+8 ;sample_x (8) reached?
instead of
movlw b'10110111' ;sample_x (7) reached?
(I think you probably want to check for 8 not 7)

using W and F instead of 1 and 0 makes it easier to read.

Maybe it should be goto $+3 (if the next two instructions are single word instructions).

Thanks! Will revise the code in post #138 in a moment but I actually asked the question for the next code to be posted!

 

This routine will be called to multiply a positive coefficient with a positive sample.
And it assumes that the two pointers point to the low byte of each when it is called.

Note: res0, res1,res2 and res3 are declared in Access RAM

Any comments will be appreciated!

Edit: I just edited a few comments! a nice exercise for me would be to simulate this function to multiply two unsigned 16 bit numbers and check if the result is always consistent... but don't really have the time for now but will definitely.

 

I don't know about post 139, hopefully someone else will.
Thinking about my last post, it should be:
movlw low(samples_x+8) ;sample_x (8) reached?

 

Just so you know how I'm planning to use those two fuctions 'muladd' and 'mulsub'

Note: This is incomplete but I realize it is much easier to code when I start with the *comments*

'*****' means I have to add the code!

Edit: The code has been edited a bit so that the 'mulladd' does not get executed after the 'mulsub' execution and it makes sure that only one function get executed!

But in order to complete this I have to finish with post #138 first but I'll ask a question in the next post

 

As for post #137 and #138 as I'm about to revise that code...

What if I keep my array (sample_x) size = 3 words (6 bytes) it's still power of 2, correct?

This way we going to have:

loop

n-0 n-2 n-1
n-1 n-0 n-2
n-2 n-1 n-0

goto loop

So our loop has 3 iterations. Thus we need to make sure we are in the correct iteration/ or state when selecting the samples! I think the incomplete multiply_function would need a flag to be notify when there is a new sample or when the sample_x array has been updated so it makes the correct selection of samples.

Will Think about it as soon as I get the time and in the meantime I will look again at Markd77 previous comments about the resetting of the pointer and the indexes.

I like this thread coz it's challenging to me ...

Note that code at #144 has been slight edited as mentioned.

 

Based on the comment below done way back I implemented the 'mulsub' routine that multiplies '-ve coefficients' with the samples that are always positive and having in mind that '-ve coefficients' had been saved as positive number (2s complement) given that this multiplier only work with positive numbers.

And before writing I simulated the algorithm manually on a paper with a pencil and the result is always consistent.

I will have to revise my digital electronics, binary arithmetics and... coz without a good knowledge of that it's difficult to come up with good algo like this one

...

1) Test 1st operand. If it is negative, 2s complement it *and remember it was negative*.

2) Test 2nd operand. 2s complement if necessary, and remember

3) Multiply

4) if either oper1 or oper2 was negative (but not both), 2s complement the result.

So you have at most 3 2s complement operations to perform with each multiplication! This consumes an extra 2+n instruction cycles per multiplication, where n is the number of bytes of the operand (or result).

I am suggesting that you keep your numbers positive, eliminating the need for the extra 2s complements, and use flags to keep track of +/-. You will still need to test signs and figure the sign of the result, but you will save time overall.

You can still have one mulacc routine. Just have it check the flags and automatically decide to add or subtract the unsigned values.
...

I am still thinking if that would be possible to have a single routine that does the multiplication for both '+ve' and '-ve' coefficients because checking the sign of the coefficient inside the routine looks messy given the structure of the routine.

I will have to write a macro 'bnc', branch on not carry, that is not on the macro list of Joey but that is no problem.

Any comment is appreciated!

 

I already spotted an error in the code...

array res:4 is not equal to res3, res2, res1 and res1.

Will try to fix that

 

As for post #137 and #138 as I'm about to revise that code...

What if I keep my array (sample_x) size = 3 words (6 bytes) it's still power of 2, correct?

This way we going to have:

loop

n-0 n-2 n-1
n-1 n-0 n-2
n-2 n-1 n-0

goto loop

So our loop has 3 iterations. Thus we need to make sure we are in the correct iteration/ or state when selecting the samples! I think the incomplete multiply_function would need a flag to be notify when there is a new sample or when the sample_x array has been updated so it makes the correct selection of samples.

Will Think about it as soon as I get the time and in the meantime I will look again at Markd77 previous comments about the resetting of the pointer and the indexes.

I like this thread coz it's challenging to me ...

Note that code at #144 has been slight edited as mentioned.

Thinking about it, it almost certainly takes less cycles overall to just shift all the values through the array each time, especially with some of the 18F instructions, eg there are 6 bytes, b0-b5
b3->b5
b2->b4
b1->b3
b0->b2
new samples into b0 and b1
If the array was maybe 50 bytes long, it would probably be quicker with the other method.

Your most recent post, I think Joey was saying to keep the numbers positive throughout the program, keeping flags for which are negative. If you later need to add them up, you can use the flags to decide if the answer is positive or negative. Two's compliment is quite annoying if you are using multiple bytes to store each number.

 

What about this:

lfsr 0,res3                       ; fsr0 -> res3 
comf postdec0,f              ;
comf postdec0,f              ;
comf postdec0,f              ;
negf postinc0                  ;
bnc done                       ; branch on not carry
incf postinc0,f                 ;
bnc done                      ; branch on not carry
incf postinc0,f               ;
skpnc                          ;
incf indf0,f                   ;

With the registers declared as follows:

res0
res1
res2
res3

Should work I think!

 

Your most recent post, I think Joey was saying to keep the numbers positive throughout the program, keeping flags for which are negative. If you later need to add them up, you can use the flags to decide if the answer is positive or negative. Two's compliment is quite annoying if you are using multiple bytes to store each number.

The main reason for keeping the numbers positive and keeping the flags for which are negative is because the PIC18F multiplier Only works with unsigned numbers otherwise we wouldn't bother doing all this.

I'm analyzing your other comment and thanks for your continued help!

 

Well I think I'll have to write a quick code and test it with MplabSim with real numbers and check the results to be 100% sure about these two routines...but don't really have time for now...

Anyway Enjoy your weekend all of you AAC members!

 

I am still thinking if that would be possible to have a single routine that does the multiplication for both '+ve' and '-ve' coefficients because checking the sign of the coefficient inside the routine looks messy given the structure of the routine.

If it helps, here's an example 16x16 signed multiply routine.

Regards, Mike

;
;  16 x 16 -> 32 signed multiply (16 bit core)
;
;  AL*BL + AH*BH*256*256 + AH*BL*256 + AL*BH*256
;
;  40 words, 43 cycles (including call and return)
;
Mult16x16
        movf    AL,W            ; W = AL			  |
        mulwf   BL              ; AL * BL
        movff   PRODL,Prod0     ; -- -- -- LL
        movff   PRODH,Prod1     ; -- -- MM --

        movf    AH,W            ; W = AH
        mulwf   BH              ; AH * BH * 256 * 256
        movff   PRODL,Prod2     ; -- HH -- --
        movff   PRODH,Prod3     ; UU -- -- --

        mulwf   BL              ; AH * BL * 256
        movf    PRODL,W         ;
        addwf   Prod1,F         ; -- -- MM --
        movf    PRODH,W         ;
        addwfc  Prod2,F         ; -- HH -- --
        movlw   0               ;
        addwfc  Prod3,F         ; UU -- -- --

        movf    AL,W            ; W = AL
        mulwf   BH              ; AL * BH * 256
        movf    PRODL,W         ;
        addwf   Prod1,F         ; -- -- MM --
        movf    PRODH,W         ;
        addwfc  Prod2,F         ; -- HH -- --
        movlw   0               ;
        addwfc  Prod3,F         ; UU -- -- --
SignB
        btfss   BH,7            ; BH:BL argument negative?
        bra     SignA           ; no, branch, else
        movf    AL,W            ;
        subwf   Prod2,F         ;
        movf    AH,W            ;
        subwfb  Prod3,F         ;
SignA
        btfss   AH,7            ; AH:AL argument negative?
        bra     Mulxit          ; no, branch, else
        movf    BL,W            ;
        subwf   Prod2,F         ;
        movf    BH,W            ;
        subwfb  Prod3,F         ;
Mulxit
        return                  ;

 

Thanks Mike for your comment!

This routine is very similar to that of the datasheet and I can confirm that the '16 x 16 signed' multiplication routine will work just fine for the '16 by 16 unsigned' multiplication. correct me if my statement is wrong. So this one routine can be used in both case.

And if I had to use that I'll only do a single sign test as, in my case, samples are always positive.

But again this assumes that negative numbers have been kept unchanged, ie they have not been made positive (2s complement) like in my case and I do not want to edit what has been done before as it works beautifully.
Also I prefer using two pointers to loading 4 different bytes for each different multiplication.

If you take a look at my 'mulladd' routine you'll see it's the same routine as that of the 16 by 16 unsigned of the datasheet but the only difference is I am using pointers.

So I think for now I will use the two different routines...the flag will be checked first and the appropriate routine will be called and move on to the next module to be implemented like the storing of the sample... this is only the 1st version...after the whole system works (Hopefully) there will be refinement there and there.

it's going to be something like:

lfsr 0,filcof1        ;fsr0-> low byte of 1st coefficient (b11) table for filter 1
 lfsr 1,samples_x         ;fsr1-> low byte of x(n)
 callbs f1flags,1,1,mulsub  ;if coefficient1 is -ve call negative multiply function
 btfss f1flags,1,1            ;otherwise call the positive one
 call muladd

The above code is incomplete but this is just to show what I have in mind...

I will be very active on this thread again from next weekend...

Thanks again

 

....
Thinking about my last post, it should be:
movlw low(samples_x+8) ;sample_x (8) reached?

This would copy the low address of the register, Wow... I never used this expression (high , low) on a register, Only on literals!

This is a good one and makes perfect sense!

 

Thinking about it, it almost certainly takes less cycles overall to just shift all the values through the array each time, especially with some of the 18F instructions, eg there are 6 bytes, b0-b5
b3->b5
b2->b4
b1->b3
b0->b2
new samples into b0 and b1
If the array was maybe 50 bytes long, it would probably be quicker with the other method.
...

This looks clever!
That will certainly decrease complexity of implementation. This way the new sample x(n) will always be put at the very same location.
I like that...Ok I'm investigating how I can implement it...

Thanks!

 

So we going to have something like this:

n-0
n-0 n-1
n-0 n-1 n-2
n-0 n-1 n-2
....

New high isr:

; high priority interrupt service routine
; Context Saving for High ISR
movff sample_x+3,sampl_x+5  ;x(n-1) -> x(n-2)
movff sample_x+2,sampl_x+4  ;​
movff sample_x+1,sampl_x+3  ;x(n) -> x(n-1)
movff sample_x,sampl_x+2     ;​
movff    adresl,sample_x        ;adresl:adresh -> x(n)
movff    adresh,sample_x+1   ;​
bsf  flag,fnew_xn,0               ;set new x(n) sample flag
 
decfsz   sample_cnt0           ;count 2000 samples (250*8=2000) 
goto     exit_high_isr            ;exit
movlf    sample_cnt0, d'250'  ;reset sample counter
decfsz   sample_cnt1
goto  exit_high_isr               ;exit
movlf    sample_cnt1, d'8'      ;reset sample counter
 
bsf  flag,smplflag,0           ;set 2000 samples flags​
exit_high_isr
retfie  fast          ;get out fast​

No more need for pointer.

High isr job so far:

- update sample array and copy new sample
- set flag for new sample
- count number of samples

Any comments?

Also can anybody show me how to format code nicely? especially comments.

we also have to take care of the Y's: y(n), y(n-1) and y(n-2)
An array will be created and updated the same way of the X's I think!
Working on it...

Thanks!

 

I need to reserve/allocate size to Y's...

Given the equation below and although coefficients are 16 bits and samples 10 bits (although I assigned 2 bytes).

Do you think we'll have a case where y(n) will be 32 bits? I don't think so!
I think Y's will always be 24 bits even after the addition...(well I might be wrong)coz I doubt I'll have a case with a coefficient = 0x FFFF and a sample = b'11 1111 1111'.
and 65535*1023 = 0x3FEFC01.

So I'll reserve 24 bits for the Y's...this implies I'll have to implement a 16bit by 24 bit multiplication when dealing with the 4th order filter...Ayyy!!

y(n) = b11*x(n) + b12*x(n-1) + b13*x(n-2) - a11*y1(n-1) - a12*y1(n-2)

Edit: Well I think I'll just assign 32 bits to Y's coz there's res0-res3 result of 16 by 16 multiplication...It Ok...

 

I think you are going to have to check if y gets too big, because if you are going to use the result in a 16 bit multiply later, it will cause problems. Without really understanding it, it seems that if you keep repeating that equation on new data, there is every chance that y can grow without any limits. Maybe the filter will still work OK if you set y to 0xFFFF if the result is greater than 0xFFFF.

 

I think you are going to have to check if y gets too big, because if you are going to use the result in a 16 bit multiply later, it will cause problems. Without really understanding it, it seems that if you keep repeating that equation on new data, there is every chance that y can grow without any limits. Maybe the filter will still work OK if you set y to 0xFFFF if the result is greater than 0xFFFF.

Yes! Still thinking about it...but post #156 is ok, right? Guess I understood you well in your post #148

 

I can't see any problems with #156, but I don't really know the 18F series.