| Summary: | `fallthrough` in parser is incorrect | ||
|---|---|---|---|
| Product: | Libre-SOC's first SoC | Reporter: | Jacob Lifshay <programmerjake> |
| Component: | Source Code | Assignee: | Jacob Lifshay <programmerjake> |
| Status: | CONFIRMED --- | ||
| Severity: | major | CC: | libre-soc-bugs, lkcl, programmerjake |
| Priority: | --- | ||
| Version: | unspecified | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| See Also: | https://bugs.libre-soc.org/show_bug.cgi?id=1177 | ||
| NLnet milestone: | --- | total budget (EUR) for completion of task and all subtasks: | 0 |
| budget (EUR) for this task, excluding subtasks' budget: | 0 | parent task for budget allocation: | |
| child tasks for budget allocation: | The table of payments (in EUR) for this task; TOML format: | ||
also, we should be using the mtspr and mfspr pseudo-code from Book III, since it is complete (In reply to Jacob Lifshay from comment #0) > from reading through all occurrences of `switch` in Power ISA v3.1B (there > aren't many), I think our parser incorrectly implements it: > mtspr: > switch (n) > case(129): see(Book_III_p975) > case(808, 809, 810, 811): > default: > stuff-here we'll have to check the spec, see what is intended, first. (In reply to Luke Kenneth Casson Leighton from comment #2) > we'll have to check the spec, see what is intended, first. I did: (In reply to Jacob Lifshay from comment #0) > This also matches > the description of how `switch` is supposed to work, which has no > falling-through (so like VB's Select, not like C's switch). |
from reading through all occurrences of `switch` in Power ISA v3.1B (there aren't many), I think our parser incorrectly implements it: mtspr: switch (n) case(129): see(Book_III_p975) case(808, 809, 810, 811): default: stuff-here our parser converts that to: switch (n) case(129): see(Book_III_p975) case(808, 809, 810, 811): fallthrough default: stuff-here which it then interprets (if it successfully detected the fallthrough keyword instead of trying to call `self.fallthrough()`) as: if n == 129: see(Book_III_p975) elif n in [808, 809, 810, 811] or True: # True for default stuff-here which is *wrong*. mtspr's description says if n is 808, 809, 810, or 811, then it *does nothing*, not tries to run the default case. This also matches the description of how `switch` is supposed to work, which has no falling-through (so like VB's Select, not like C's switch). so, in other words, it should be: if n == 129: see(Book_III_p975) elif n in [808, 809, 810, 811]: pass # not fallthrough else: stuff-here I propose we remove all `fallthrough` logic completely and change p_switch_case to make `suite` optional, replacing omitted `suite` with `pass`: (untested code, but should work) def p_switch_case(self, p): """switch_case : CASE LPAR atomlist RPAR COLON | CASE LPAR atomlist RPAR COLON suite """ if len(p) == 6: suite = [ast.Pass()] else: suite = p[6] p[0] = (p[3], suite)