|
|
View previous topic :: View next topic |
Author |
Message |
jdean
Joined: 28 Jun 2018 Posts: 10
|
PCD switch statements |
Posted: Thu Nov 15, 2018 10:59 am |
|
|
Greetings all!
I'm looking at using a large switch statement for a binary parser using the PCD 5.081 compiler. After reading about some of the tricks to get the PCH compiler to generate jump tables for switch statements here on the forum, I was hopeful I'd have similar luck with PCD.
I've run tests trying different types in the switch statement (8/16bit, signed/unsigned), omitting the default case, different numbers of cases (12, and 500), and tried substituting breaks for the return statements I was initially using.
Every single switch statement I've seen generated uses the same XOR ###, BZ ### approach for every case. Fine for a small number of cases, but I'm hoping for something better suited to a large set of cases.
Has anyone been able to get any other behavior out of switch/case on PCD?
Any observations or thoughts are appreciated
Code: | int1 found = 0;
switch(symb) {
case REP_BADPARSE: *args=1;*flags=0;found=1;break;
case REP_BADSEQUENCE: *args=2;*flags=0;found=1;break;
case REP_UNEXPECTEDEND: *args=0;*flags=0;found=1;break;
...
case BLAH500: *args=0;*flags=0;found=1;break;
}
|
|
|
|
jdean
Joined: 28 Jun 2018 Posts: 10
|
|
Posted: Thu Nov 15, 2018 3:21 pm |
|
|
*** note, the case values in the previous post are sequential enum values, placed in order. Presently both the enum and switch statement are generated by a separate tool from a specification.
Alright! So, proceeding under the assumption that this is the only output PCD will generate for switch statements, I'm now trying to generate a table-based switch construct.
Since the full set of cases are known by the generator, the idea is to produce a define which will goto a label corresponding to the enum.
Easy right? Well... it appears that relative branches aren't supported by the #asm directive. Absolute branches compile fine, but a relative branch in the form of "BRA W4" produces "Expecting an opcode mnemonic"
Surprising, since this particular form is both supported by my chip, and in the CCSC documentation under the "#asm" topic.
Am I on my own? |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19513
|
|
Posted: Fri Nov 16, 2018 2:13 am |
|
|
Remember there is nothing to stop you having an array of functions.
Call the one specified by an index. Instant call table. Call the functions something like case10 etc., and it can be constructed very quickly probably with a macro.
Talk to CCS about PCD not using a table. I can understand them not switching to this, for a lot more cases than in PCH/PCM, since the PIC24/30/33 are much more efficient at handling the test and branch, but they should still switch to the table form at a larger number of cases.
If you study the branch they use, they don't individually test every case. The tests are done in 'segments', and so only a limited number of tests will be used for any value. The actual speed difference to a table will be very small indeed.
Attached a crude demo of a 'case' table:
Code: |
//Use as case value, and functions to use
#define C(n, x) void C##n(void) {x}
//Crude demo showing a 'case' table. Here just incrementing/setting the case value
C(0, val++;)
C(1, val+=2;val--;) //demo of two statements
C(2, val=3;)
C(3, val++;)
C(4, val=0;)
void main()
{
typedef void (*voidfunc)(void);
voidfunc array[5] = {C0, C1, C2, C3, C4};
while(TRUE)
{
array[val]();
}
}
|
Now obviously there is an issue (displayed here), that since the functions are
separate, any variables changed have to be global.
You can also declare this as 'const', if you add #device PSV=16 to your code (this allows a PIC23/30/33 to read a window into ROM as if it is in the RAM space). |
|
|
jdean
Joined: 28 Jun 2018 Posts: 10
|
|
Posted: Fri Nov 16, 2018 12:15 pm |
|
|
Ttelmah: It looks like the array of function pointers is probably going to win out here.
I've resumed testing the switch statements as I've never seen the compiler produce either a jump table, or even a segmented approach to identifying a match. Additionally, CCS support has assured me that the compiler will choose a jump table when cases are "relatively consecutive" and large enough in number. However, with enough cases, then overall function size becomes a limit as the compile will fail once the function exceeds a segment which appears to be 64K (bytes or words, haven't confirmed which)
Independent functions should avoid that specific limitation. Many thanks for the suggestion.
Finishing my avenue of switch investigation, I've now tested up to 2964 cases, which was the most my particular test case could fit into the 64K segment. What's interesting is the compiler stopped using XOR/BRA Z once 1025 cases were hit. Above that point, it switched to CP/BRA Z, which is actually worse. The first 32 cases are CP/BRA for 2 instructions each, and remaining cases get MOV/CP/BRA for 3 each (CP only allows up to 0x1F for the integer literal) In all cases, BRA Z is used to test for equality, no range checks appear to be generated.
For both XOR and CP, there is exactly one test per case, and all branch targets are the case statements. Segmenting the tests should require additional tests, which is fine, but I'm simply not seeing any generated.
I've also tested this behavior across 5.081, 5.078 and 5.065 and the results are identical. At this point, I'm guessing maybe this is due to the specific chip family, dsPIC33E.
In case it is of help to anyone, I did have some success implementing a workaround. Though with the function size limit, and issues due to differences between dsPIC33E and dsPIC33F series, I'll be abandoning it for now. My workaround wound up looking like this:
Code: |
#define PARSER_BRANCH(target) \
unsigned int16 jmpoffs = target; \
if(jumpoffs >= NUM_CMD_IDS) { \
goto lbl_INVALID; \
} \
jumpoffs = jumpoffs<<1; \
#asm asis \
MOV jmpoffs, W0 \
BRA W0 \
#endasm \
goto lbl_BADPARSE; \
goto lbl_BADSEQUENCE; \
goto lbl_UNEXPECTEDEND; \
goto lbl_REMOTE; \
goto lbl_RELEASE; \
goto lbl_INVALID; \
goto lbl_INVALID; \
goto lbl_INVALID; \
goto lbl_LASTCASE;
void validate_symargs(int16 symbol) {
PARSER_BRANCH(symbol)
lbl_REMOTE:
lbl_LASTCASE:
return;
lbl_BADPARSE:
lbl_BADSEQUENCE:
lbl_UNEXPECTEDEND:
lbl_RELEASE:
lbl_INVALID:
return;
}
|
In my case, the define was generated by our separate tool, and both maintained clean separation between generated and hand-written code, and also resulted in meaningful errors in the event that a label was missing.
I'll continue working with support regarding the switch behavior, and update here if we make any progress. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19513
|
|
Posted: Fri Nov 16, 2018 12:44 pm |
|
|
Only 'caveat' with this is your PARSER_BRANCH is a define, so not actually called. Where will the returns take you?. |
|
|
jdean
Joined: 28 Jun 2018 Posts: 10
|
|
Posted: Fri Nov 16, 2018 1:23 pm |
|
|
To the site which called 'validate_symargs'
The define was created as a way to reuse the construct in different functions. Since these are goto/label pairs, they are scoped to the function where the PARSER_BRANCH define was applied.
I'm playing with the func ptr array you suggested, is PSV the only way to make that array const? I tried enabling it and went from 20% ram used to "Not enough RAM for all variables", and that was with a single entry in the array.
*edit* looks like the error about ram for all variables is due to other const & rom arrays that exist in the project. I came across another post of yours discussing PSV which explained that fairly well. I'm currently looking at using a const array without PSV, which is giving me errors, but I'm hopeful it's just my type signature. Will report back if I get stuck.
Last edited by jdean on Fri Nov 16, 2018 1:36 pm; edited 1 time in total |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19513
|
|
Posted: Fri Nov 16, 2018 1:35 pm |
|
|
PSV should not reduce your RAM.
It allows a ROM window to be created into the RAM address space. It doesn't use any RAM itself.
What is your chip?.
You need to enable it right at the start (immediately after the processor include). Adding it later can cause issues.
It costs a little ROM, since every ROM construct then uses an instruction word for every two bytes.
It does though allow you to use const data in places requiring pointers. |
|
|
jdean
Joined: 28 Jun 2018 Posts: 10
|
|
Posted: Fri Nov 16, 2018 3:13 pm |
|
|
right, the chip is a dsPIC33EP512MU810, my bad
I haven't dug too far into why enabling PSV gave me issues, but I was able to use a const array by first copying the function pointer into ram. I'm happy enough with that, so I'm off to the races.
PSV looks like a very interesting feature, and while I likely won't dig into it right away, I'm sure I'll get around to it. Thanks for bringing it to my attention. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19513
|
|
Posted: Sat Nov 17, 2018 2:09 am |
|
|
It may be a compiler issue, which depends on the exact 'nature' of what you have declared with rom and const.
I had some significant problems earlier this year, and after a lot of investigation CCS fixed these. 5.081, should have the fixes, but it's possible that other data patterns may still be giving problems. CCS said to me only a few days agao:
Quote: |
Yes, we are in the process of reworking the way we allocate ROM to better
work with multiple compilation units so our internal version does have some
bugs. Expect a more stable version next week.
|
So it looks as if the next release is changing things in this area. Watch this space!... |
|
|
|
|
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
|