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

Declaring kDigits static to avoid a codegen bug in VS2017. #51

Merged
merged 1 commit into from
Sep 23, 2017

Conversation

mwinterb
Copy link
Contributor

The issue should be fixed shortly, but until then.
https://developercommunity.visualstudio.com/comments/114235/view.html

@mwinterb
Copy link
Contributor Author

This seemed too noisy for the commit message, but clang generates the same code regardless of static or not. gcc generates different code: with static references a global array, without static, gcc builds up the array from several immediates. I'm not in a position to to judge which is better, but since Parse02d isn't really called on a hotpath, the differences are hopefully irrelevant.
https://godbolt.org/g/dkGFm7

@devjgm
Copy link
Contributor

devjgm commented Sep 23, 2017

Looks good. Thanks for the fix here and the bug report to Microsoft.

@devjgm devjgm merged commit d5e227e into google:master Sep 23, 2017
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