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

recognize assigning .init of all zeros, replace with memset #10967

Merged
merged 1 commit into from
Jun 11, 2020

Conversation

WalterBright
Copy link
Member

@WalterBright WalterBright commented Mar 27, 2020

Structs are often initialized or assigned values of all 0 bits. This usually results in copying the .init global into the struct. This PR recognizes some of those cases and replaces them with memset'ing them to zero. This is faster, and a step towards eliminating the .init global altogether.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#10967"

@WalterBright
Copy link
Member Author

@WalterBright WalterBright force-pushed the initZero branch 5 times, most recently from 426f24f to f508a38 Compare March 27, 2020 08:42
Comment on lines +3146 to +3142
if ((sle.useStaticInit ||
sle.elements && allZeroBits(*sle.elements) && !sle.sd.isNested()) &&
ae.e2.type.isZeroInit(ae.e2.loc))
Copy link
Member

Choose a reason for hiding this comment

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

Should this be

(sle.useStaticInit && ae.e2.type.isZeroInit(ae.e2.loc))
||
(sle.elements && allZeroBits(*sle.elements) && !sle.sd.isNested())

?

Copy link
Member Author

Choose a reason for hiding this comment

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

no because sle.elements does not include default initializers.

@WalterBright WalterBright force-pushed the initZero branch 2 times, most recently from 2907d00 to 02962c5 Compare May 15, 2020 08:13
@WalterBright
Copy link
Member Author

C++ interop tests / Run (macOS-10.15, clang-4.0.0) (pull_request) Failing after 46m

So I click for "Details", and there are no details, no log, just it failed.

@ghost
Copy link

ghost commented Jun 10, 2020

Does this include init with void gaps ?, e.g

struct F { int i; void* p = void; int j; }

because in this case the gap can be ignored and filled with 0 as well.

@WalterBright
Copy link
Member Author

Looks like the C++ heisenbug disappeared.

@dlang-bot dlang-bot merged commit 0349239 into dlang:master Jun 11, 2020
@WalterBright WalterBright deleted the initZero branch June 11, 2020 22:30
@WalterBright
Copy link
Member Author

@atilaneves wunderbar!

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

Successfully merging this pull request may close these issues.

4 participants