Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for computed goto #874

Merged
merged 12 commits into from Apr 21, 2019
Merged

Support for computed goto #874

merged 12 commits into from Apr 21, 2019

Conversation

@clbr
Copy link
Contributor

@clbr clbr commented Apr 10, 2019

Depends on #873, and includes those commits so that the autobuilder will succeed.
Closes #864.

Copy link
Contributor

@oliverschmidt oliverschmidt left a comment

This looks like a rather complex change to me - and I don't understand it's importance.

@greg-king5: I'd appreciate your opinion!

Loading

src/cc65/codeseg.c Outdated Show resolved Hide resolved
Loading
src/cc65/expr.c Outdated Show resolved Hide resolved
Loading
src/cc65/goto.c Outdated Show resolved Hide resolved
Loading
src/cc65/goto.c Outdated Show resolved Hide resolved
Loading
src/cc65/goto.c Outdated Show resolved Hide resolved
Loading
src/cc65/goto.c Outdated Show resolved Hide resolved
Loading
src/cc65/symtab.c Outdated Show resolved Hide resolved
Loading
@clbr
Copy link
Contributor Author

@clbr clbr commented Apr 13, 2019

Will fix the missing spaces, thanks.

As mentioned in #864, it allows efficient jump tables from C. A switch-case or a cascade of ifs can take 30x or more cpu vs what a jump table jump takes, and it also changes the timing from variable to constant. As a plus, it's also smaller than the alternatives.

Loading

src/cc65/expr.c Outdated Show resolved Hide resolved
Loading
src/cc65/goto.c Outdated Show resolved Hide resolved
Loading
src/cc65/goto.c Outdated Show resolved Hide resolved
Loading
src/cc65/goto.c Outdated Show resolved Hide resolved
Loading
test/val/computedgoto.c Outdated Show resolved Hide resolved
Loading
test/val/jmp-callax.c Outdated Show resolved Hide resolved
Loading
test/val/jmp-callax.c Outdated Show resolved Hide resolved
Loading
@clbr
Copy link
Contributor Author

@clbr clbr commented Apr 14, 2019

That test shouldn't be done if the previous test says that the variable isn't an array.

Isn't that what the Error does? Stops execution since we can't compile this.

Loading

@greg-king5
Copy link
Contributor

@greg-king5 greg-king5 commented Apr 15, 2019

That test shouldn't be done if the previous test says that the variable isn't an array.

Isn't that what Error() does: stops execution since we can't compile this?

Warning() and Error() show diagnostics on stderr, and count them. Compilation continues, so that other problems can be found. At the end, the output file is written only if no errors were counted.

Loading

@clbr
Copy link
Contributor Author

@clbr clbr commented Apr 15, 2019

I checked the source; it's only fatal after a few errors, my mistake, I remembered it was fatal instantly.

Loading

src/cc65/goto.c Outdated Show resolved Hide resolved
Loading
src/cc65/goto.c Show resolved Hide resolved
Loading
src/cc65/goto.c Outdated Show resolved Hide resolved
Loading
src/cc65/codeseg.c Show resolved Hide resolved
Loading
@clbr
Copy link
Contributor Author

@clbr clbr commented Apr 15, 2019

About the "only for jumps" message, expanding it slightly:
"Do the test only for jumps, because the lib code has manual asm statements like asm("bne L1"). These L1-named labels don't match the IsLocalLabel function, and not emitting that label would break the function".

Loading

@polluks
Copy link
Contributor

@polluks polluks commented Apr 15, 2019

Maybe a hint in cc65.html#s5?

Loading

@oliverschmidt
Copy link
Contributor

@oliverschmidt oliverschmidt commented Apr 20, 2019

[...] and I don't understand it's importance.

Now with the docs addition I get it...

@greg-king5: Thanks for your review! As far as I can see this pr is ready to be merged from your pov, right? In that case please either acknowledge or merge right away.

@polluks: Thanks for the hint on the docs.

Loading

@greg-king5
Copy link
Contributor

@greg-king5 greg-king5 commented Apr 21, 2019

As mentioned in #864, it allows efficient jump tables from C. A switch-case or a cascade of ifs can take 30x or more CPU time vs what a jump-table jump takes. And, it also changes the timing from variable to constant. As a plus, it's also smaller than the alternatives.

Those advantages are useful especially when making game animations.

Merging ...

Loading

@greg-king5 greg-king5 merged commit 55ce618 into cc65:master Apr 21, 2019
1 check passed
Loading
@clbr clbr deleted the computedgoto branch Apr 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants