This is an archive of the discontinued LLVM Phabricator instance.

[RISCV][NFC] Move static global variables into static variable in function.
ClosedPublic

Authored by kito-cheng on Jun 28 2022, 6:15 AM.

Details

Summary

It's violate coding guideline in LLVM coding standard[1], because the the initialization order is nondeterministic and that might increase the launch time of programs.

However these variables are only used to cache query result, so we can move these variables into the function,, that which resolve both issue: 1. initialized in deterministic order, 2. Initialized that when the first time used.

[1] https://llvm.org/docs/CodingStandards.html#do-not-use-static-constructors

Diff Detail

Event Timeline

kito-cheng created this revision.Jun 28 2022, 6:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2022, 6:15 AM
kito-cheng requested review of this revision.Jun 28 2022, 6:15 AM

The commit message is poor, because the problem is not the use of static globals, it's the use of globals that have constructors. Moving them to be static function-scoped variables doesn't change the fact that they still have static duration; you change the scope of them but not their storage. Implementations can defer their initialisation until the first time the function is called, but do not have to. Clang, GCC and MSVC all do in fact perform lazy initialisation (with all the overhead that comes with to make it thread-safe and exception-safe) so maybe this is fine in practice. Is there precedent for this?

My understanding is the reason why no global variable is because 1. the initialization order and 2. might increase the launch time of programs, moving that into function scope could resolve both issue: 1. initialized in deterministic order[1], 2. Initialized that when the first time used.

[1] https://stackoverflow.com/questions/49856152/static-function-variable-initialization-order-in-the-same-function
[2] https://en.cppreference.com/w/cpp/language/storage_duration#Static_local_variables

  • stackoverflow and cppreference might not formal source, but should be enough here.
kito-cheng edited the summary of this revision. (Show Details)Jun 29 2022, 6:55 AM
jrtc27 added a comment.EditedJun 29 2022, 6:58 AM

Seems you're right, the C++11 standard does clearly say (9.7p4):

Dynamic initialization of a block-scope variable with static storage duration (6.6.4.1) or thread storage duration (6.6.4.2) is performed the first time control passes through its declaration; such a variable is considered initialized upon the completion of its initialization. If the initialization exits by throwing an exception, the initialization is not complete, so it will be tried again the next time control enters the declaration. If control enters the declaration concurrently while the variable is being initialized, the concurrent execution shall wait for completion of the initialization. If control re-enters the declaration recursively while the variable is being initialized, the behavior is undefined.

Not sure why I thought it was just an optimisation then.

MaskRay accepted this revision.Jun 29 2022, 4:12 PM

It's violate coding guideline in LLVM coding standard[1], because the the initialization order is nondeterministic and that might increase the launch time of programs.

nondeterministic => indeterminate

This revision is now accepted and ready to land.Jun 29 2022, 4:12 PM
This revision was landed with ongoing or failed builds.Jun 29 2022, 7:30 PM
This revision was automatically updated to reflect the committed changes.