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

Use modern enums in compiler #15775

Merged
merged 10 commits into from
Nov 2, 2020
Merged

Use modern enums in compiler #15775

merged 10 commits into from
Nov 2, 2020

Conversation

cooldome
Copy link
Member

Reduce number of constant strings, but reusing string representation of nim enums that compiler will generate anyway.
I managed to shave off 35 kb from the compiler binary with this trick.

I haven't noticed a slowdown in compiler despite I know about reprEnum string allocation overhead.
Possibly we need to improve codegen for reprEnum before making this change.

Also I think we need to get rid of findStr proc in wordrecg in favour of parseEnum. It is efficient now

@cooldome cooldome changed the title Use morden enums in compiler Use mordern enums in compiler Oct 29, 2020
@cooldome cooldome changed the title Use mordern enums in compiler Use modern enums in compiler Oct 29, 2020
@Clyybber
Copy link
Contributor

I did similar things in #14777 (comment), see Araqs comment for why I reverted those changes in that PR.

@timotheecour
Copy link
Member

That's now 3 people (i had also attempted a similar thing some time ago) that have tried to replace those array[Enum, string]. Unfortunately we're going in circles. See #14777 (comment)

I don't like the enumValue = "string repr" pattern

Can we then reconsider #14008 ?

Yeah, that's a nice compromise. Bring it on please!

but then it got closed (along with its fusion alternative).

@Araq
Copy link
Member

Araq commented Oct 30, 2020

That's now 3 people

And that's my threshold so I'm willing to accept this solution now.

@cooldome
Copy link
Member Author

Looks good now, I will look into speeding up reprEnum in future PRs

const CallingConvToStr*: array[TCallingConvention, string] = ["nimcall", "stdcall",
"cdecl", "safecall", "syscall", "inline", "noinline", "fastcall", "thiscall",
"closure", "noconv"]
ccNimCall = "nimcall" # nimcall, also the default
Copy link
Member

@timotheecour timotheecour Oct 30, 2020

Choose a reason for hiding this comment

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

unfortunately this still leaves CallingConvToStr: array[TCallingConvention, string] = ["N_NIMCALL",... in compiler/ccgtypes.nim but without
https://github.com/nim-lang/Nim/pull/14008/files#diff-6b7c28355957ab974fa38584bc810d1aac557bdcc7874e98d8ec005cc2200d72 that's the best one can do; still a good improvement.

@Araq Araq merged commit 00b495d into devel Nov 2, 2020
@Araq Araq deleted the use_morden_enums branch November 2, 2020 09:35
narimiran pushed a commit that referenced this pull request Nov 9, 2020
PMunch pushed a commit to PMunch/Nim that referenced this pull request Jan 6, 2021
mildred pushed a commit to mildred/Nim that referenced this pull request Jan 11, 2021
irdassis pushed a commit to irdassis/Nim that referenced this pull request Mar 16, 2021
ardek66 pushed a commit to ardek66/Nim that referenced this pull request Mar 26, 2021
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.

4 participants