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

Added support for compiling functions inline #69

Merged
merged 12 commits into from
Jan 7, 2025

Conversation

smartycope
Copy link
Contributor

"inline" coming from the C++ keyword that does the same thing

Copy link
Owner

@Lonami Lonami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make more sense to have a builtin @inline decorator that can be applied on a function-by-function basis, instead of a global switch?

pyndustric/__main__.py Outdated Show resolved Hide resolved
Co-authored-by: Lonami <[email protected]>
@smartycope
Copy link
Contributor Author

Actually, yes, that's a great idea. I like that. I'll work on adding that instead.
As a side note, should we keep the ability to add kwargs to the compiler? It seems like that could be useful in the future.

@Lonami
Copy link
Owner

Lonami commented Aug 17, 2024

should we keep the ability to add kwargs to the compiler?

I'm not sure. I tend to prefer not having features we're not using. If you have any use cases in mind for the near future, sure. Otherwise I'd be inclined to remove it, as it also looks kind of funny in the tests, with all the kwargs and inspect usage.

@smartycope
Copy link
Contributor Author

Fair. I was thinking for specifying functions to run at the end instead of using a stack pointer in a memory block, but we could just use function decorators for that as well.
I guess it could be useful for optimizers? Or specifying non-default options, like changing whether we use a stack or not?

@Lonami
Copy link
Owner

Lonami commented Aug 18, 2024

specifying functions to run at the end

This is an optimization to avoid needing the jump x always to skip the function. It'd save an instruction per function, and with how constrainted Mindustry processors are, I don't imagine anyone wanting to turn this off ^^

using a stack pointer in a memory block

That's something else. This is needed to know where to return to. I don't think it's something we can remove. As soon as a function can call another function, you need a list of positions to return to. A single variable won't do, and sadly we can't refer to variables by a dynamic name.

This can be removed for function parameters, as long as they are not recursive. But then again, I think this would be another decorator and not a global switch (perhaps assume functions don't recurse, and opt-in via @recursive, because not supporting recursion is cheaper).

be useful for optimizers

Do you mean different optimizer levels? I don't think pyndustric would reach the level of maturity where this matters. Even then, optimizing for size is probably the right call in most circumstances, again due to the game's limit.

changing whether we use a stack or no

I could see a flag to make the compiler fail if it can't produce code without having a stack available (because in some cases it does need one). But, the user can also just see if the output contains cell, so I don't know.


Sadly in the order PRs are being merged this one has conflicts now. Please open or mark others as draft, if you'd prefer I don't merge those yet.

Copy link
Contributor Author

@smartycope smartycope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you're right. I just keep thinking of the C++ compiler, and how many hundreds of command line parameters it has. I guess I can remove it, we can always just re-add it if it becomes relevant later.

README.md Outdated Show resolved Hide resolved
pyndustri.pyi Outdated Show resolved Hide resolved
@smartycope smartycope force-pushed the Inline-functions branch 2 times, most recently from 31463ad to 8646703 Compare January 3, 2025 23:42
@Lonami
Copy link
Owner

Lonami commented Jan 4, 2025

This branch cannot be rebased due to conflicts

Do you want to fix those or is merge OK?

pyndustric/compiler.py Outdated Show resolved Hide resolved
@smartycope
Copy link
Contributor Author

I think it's good to merge now. I don't have write access, so you'll have to do it.

@Lonami Lonami merged commit ed202db into Lonami:master Jan 7, 2025
2 checks passed
@Lonami
Copy link
Owner

Lonami commented Jan 7, 2025

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants