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

Skip optimizing variables started with @ #114

Closed
wants to merge 1 commit into from

Conversation

whir1
Copy link

@whir1 whir1 commented Oct 21, 2021

this PR allows set user defined variable names after compile nolol to yolol.

It's need when i preffered custom variable names insted of a,b,c, and in some cases to generate more compact code.
For example: when i tried port ISAN yasm code to nolol code the result code are not optimal because yasm compiler generate

t=p-:a t*=t i=p-:b i*=i g=p-:c g*=g f=p-:d f*=f x/=:a*:b*:c*:d goto14

and
nolol compiler generate

ag=af-:a ag*=ag ah=af-:b ah*=ah ai=af-:c ai*=ai aj=af-:d aj*=aj
t/=:a*:b*:c*:d goto15

because ISAN has many params

Which issue(s) this PR fixes**:

It's new feature i don't make it =D

Sources:

If this PR aims to improve compatibility to the game's implementation (or another existing YOLOL-implementation), please state why you know that the game does it this way.
(E.g. you tested it ingame yourself, you read it in the wiki etc.)

Checklist:

  • PR is done against develop-branch
  • includes tests for everything that changed
  • updated documentation (if necessary)

@dbaumgarten
Copy link
Owner

ISAN is a really hard case. It is massively hand-optimized to fit exactly into the lines.
There have been suggestions to tackle the issue with non-optimal variable-shortening in a more general way.
One suggestion was to give variables that are used often shorter optimized names. That would not be an optimal solution, but better then the current one.

You approach is interesting, as it makes a few edge-cases possible. But I would rather see the compiler handle something like this automatically. Programmers should not need to worry about the names for local variables at all.

So, while I see the use for your change, I am hesitant to add this as a feature, because it could be soon superseeded by a more general solution. Let me think about this a little.

@dbaumgarten
Copy link
Owner

@whir1 I have just pushed some changes to develop, which make the compiler optimize variables based on how often they appear in the source-code. That should help with script that use many local variables.

You can find the automated build of develop here: https://github.com/dbaumgarten/yodk/releases/tag/latest
Would you like to test how this version affects your ISAN-port? That would be really interesting.

@whir1
Copy link
Author

whir1 commented Oct 24, 2021

https://gist.github.com/whir1/07edb18b1bb96ddc3b483fe36a288f1f

It's better, but not enoth =D.
When i tried compile with fixed layout then i get error =\

One suggestion was to give variables that are used often shorter optimized names.

It's not a bad idea for many cases, but for my opinion we should calculate line which use maximum count of variables, and this vars should get one letter names.

ISAN is a really hard case. It is massively hand-optimized to fit exactly into the lines.

Yes, and it's very intresting to port it to nolol instead yasm =D

Programmers should not need to worry about the names for local variables at all.

maybe you right but when you look to ofuscated and shortened result code in game you eyes will cry =D
My approach that not all script should optimize variable names because it not nesessary in many cases. And in some cases variable names should look exactly the way i want/ For example uwp or xyz vector or i, j counters names

@dbaumgarten
Copy link
Owner

Damn, I hoped this would be enough.

The issue is, that the variable-names are replaced before the final line-layout is done. So by the time you know which line has how many variables it is already to late. (And it has to be this way. Without replacing the variable-names the compiler can't be sure if he can merge lines).

I am not sure if, in it's current state, NOLOL is up to this task. ISAN might be one of the few cases that just need to be hand-coded.

I guess looking at the output-code will always hurt. Thats the case for any compiler. (And the better the compiler does it's job, the worse this gets). The code will be ugly, and making some variables look nice won't really help. You'd be better of, ignoring compiled code and just work with the original code.

@dbaumgarten
Copy link
Owner

Currently I don't really want to merge this for multiple reasons.

  1. It is a real edge-case barely anyone will need
  2. It would require more work (documentation, syntax-highlighting, avoiding collisions during renaming etc.)
  3. This is so low-level that programmers should not even deal with it
  4. I already have another use in mind for the @ sign
  5. Hopefully further compiler-improvements make this feature obsolete

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