Skip to content

Conversation

@lanza
Copy link
Member

@lanza lanza commented Dec 3, 2025

Stack from ghstack (oldest at bottom):

This implements thread-safe initialization of function-local static
variables with dynamic initializers, following the Itanium C++ ABI.

The implementation:

  1. Adds static_local attribute to GlobalOp and GetGlobalOp to mark
    function-local statics requiring guarded initialization
  2. Implements guarded initialization in LoweringPrepare pass using
    __cxa_guard_acquire/__cxa_guard_release for thread safety
  3. Adds helper methods createIsNull/createIsNotNull to CIRBaseBuilder
  4. Implements emitGuardedInit in CIRGenItaniumCXXABI to emit the
    ctor region and mark the global as static_local
  5. Adds mangleStaticGuardVariable to ASTAttrInterfaces for mangling

The guard variable pattern follows Itanium ABI 3.3.2:

  • First byte of guard checked with acquire load for fast path
  • __cxa_guard_acquire called if uninitialized
  • __cxa_guard_release called after successful initialization

[ghstack-poisoned]
lanza added a commit that referenced this pull request Dec 3, 2025
…bles

This implements thread-safe initialization of function-local static
variables with dynamic initializers, following the Itanium C++ ABI.

The implementation:
1. Adds `static_local` attribute to GlobalOp and GetGlobalOp to mark
   function-local statics requiring guarded initialization
2. Implements guarded initialization in LoweringPrepare pass using
   __cxa_guard_acquire/__cxa_guard_release for thread safety
3. Adds helper methods createIsNull/createIsNotNull to CIRBaseBuilder
4. Implements emitGuardedInit in CIRGenItaniumCXXABI to emit the
   ctor region and mark the global as static_local
5. Adds mangleStaticGuardVariable to ASTAttrInterfaces for mangling

The guard variable pattern follows Itanium ABI 3.3.2:
- First byte of guard checked with acquire load for fast path
- __cxa_guard_acquire called if uninitialized
- __cxa_guard_release called after successful initialization


ghstack-source-id: 6081423
Pull-Request: #2046
[ghstack-poisoned]
lanza added a commit that referenced this pull request Dec 3, 2025
…bles

This implements thread-safe initialization of function-local static
variables with dynamic initializers, following the Itanium C++ ABI.

The implementation:
1. Adds `static_local` attribute to GlobalOp and GetGlobalOp to mark
   function-local statics requiring guarded initialization
2. Implements guarded initialization in LoweringPrepare pass using
   __cxa_guard_acquire/__cxa_guard_release for thread safety
3. Adds helper methods createIsNull/createIsNotNull to CIRBaseBuilder
4. Implements emitGuardedInit in CIRGenItaniumCXXABI to emit the
   ctor region and mark the global as static_local
5. Adds mangleStaticGuardVariable to ASTAttrInterfaces for mangling

The guard variable pattern follows Itanium ABI 3.3.2:
- First byte of guard checked with acquire load for fast path
- __cxa_guard_acquire called if uninitialized
- __cxa_guard_release called after successful initialization

ghstack-source-id: bf90e4f
Pull-Request: #2046
[ghstack-poisoned]
lanza added a commit that referenced this pull request Dec 3, 2025
…bles

This implements thread-safe initialization of function-local static
variables with dynamic initializers, following the Itanium C++ ABI.

The implementation:
1. Adds `static_local` attribute to GlobalOp and GetGlobalOp to mark
   function-local statics requiring guarded initialization
2. Implements guarded initialization in LoweringPrepare pass using
   __cxa_guard_acquire/__cxa_guard_release for thread safety
3. Adds helper methods createIsNull/createIsNotNull to CIRBaseBuilder
4. Implements emitGuardedInit in CIRGenItaniumCXXABI to emit the
   ctor region and mark the global as static_local
5. Adds mangleStaticGuardVariable to ASTAttrInterfaces for mangling

The guard variable pattern follows Itanium ABI 3.3.2:
- First byte of guard checked with acquire load for fast path
- __cxa_guard_acquire called if uninitialized
- __cxa_guard_release called after successful initialization

ghstack-source-id: 272ef4e
Pull-Request: #2046
@lanza lanza marked this pull request as draft December 3, 2025 16:57
[ghstack-poisoned]
lanza added a commit that referenced this pull request Dec 5, 2025
…bles

This implements thread-safe initialization of function-local static
variables with dynamic initializers, following the Itanium C++ ABI.

The implementation:
1. Adds `static_local` attribute to GlobalOp and GetGlobalOp to mark
   function-local statics requiring guarded initialization
2. Implements guarded initialization in LoweringPrepare pass using
   __cxa_guard_acquire/__cxa_guard_release for thread safety
3. Adds helper methods createIsNull/createIsNotNull to CIRBaseBuilder
4. Implements emitGuardedInit in CIRGenItaniumCXXABI to emit the
   ctor region and mark the global as static_local
5. Adds mangleStaticGuardVariable to ASTAttrInterfaces for mangling

The guard variable pattern follows Itanium ABI 3.3.2:
- First byte of guard checked with acquire load for fast path
- __cxa_guard_acquire called if uninitialized
- __cxa_guard_release called after successful initialization

ghstack-source-id: 29329f1
Pull-Request: #2046
@lanza lanza marked this pull request as ready for review December 5, 2025 06:47
Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

Nice, this looks mostly good. Please add a .cir tests for parsing/printing. More comments inline

oldGlobalOp.getName(), typedInit.getType(),
oldGlobalOp.getConstant(), globalOp.getLinkage());
// FIXME(cir): OG codegen inserts new GV before old one, we probably don't
// need that?
Copy link
Member

Choose a reason for hiding this comment

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

LLVM translation usually put them out in the proper order, because of the way MLIR handles this I don't think it should be a problem for us.

void emitCXXGuardedInit(const VarDecl &varDecl, cir::GlobalOp globalOp,
bool performInit);

/// EmitCXXGlobalVarDeclInit - Create the initializer for a C++
Copy link
Member

Choose a reason for hiding this comment

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

Remove the EmitCXXGlobalVarDeclInit - in the comment

// CIR: cir.global "private" internal dso_local static_local @_ZZ3foovE3val = #cir.int<0> : !s32i {alignment = 4 : i64, ast = #cir.var.decl.ast}
// CIR: cir.global "private" internal dso_local @_ZGVZ3foovE3val = #cir.int<0> : !s64i {alignment = 8 : i64}
// CIR: cir.func {{.*}} @_Z3foov() extra(#fn_attr) {
// CIR-NEXT: %0 = cir.get_global static_local @_ZZ3foovE3val : !cir.ptr<!s32i>
Copy link
Member

Choose a reason for hiding this comment

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

None of our tests should have literal SSA values, regex it all please!

// CIRGEN-NEXT: %1 = cir.call @_Z3fnAv() : () -> !s32i
// CIRGEN-NEXT: cir.store align(4) %1, %0 : !s32i, !cir.ptr<!s32i>
// CIRGEN-NEXT: } {alignment = 4 : i64, ast = #cir.var.decl.ast}
// CIRGEN-NEXT: cir.func
Copy link
Member

Choose a reason for hiding this comment

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

The check for cir.func should only be CIRGEN since it's a coincidence it shows up next but it isn't required to do so. Also add the name of the function too.


// CIRGEN: cir.func private @_Z3fnAv() -> !s32i
// CIRGEN: cir.global "private" internal dso_local static_local @_ZZ3foovE3val = ctor : !s32i {
// CIRGEN-NEXT: %0 = cir.get_global @_ZZ3foovE3val : !cir.ptr<!s32i>
Copy link
Member

Choose a reason for hiding this comment

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

Having two instances of cir.get_global @_ZZ3foovE3val inside the initializer and another one outside with the specific keyword is a bit confusing, however still pretty handy when unwrapping so that lowering prepare could be more direct.

Since there's still more work in this area, better not over design too early, but we should revisit this in the future. Can you add a comment in the CIRGen code that generates this get_global so we can later take other things into consideration?

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