Skip to content

CIRGen support for *_atomic_{thread,signal}_fence #1274

Closed
@bcardosolopes

Description

All the below are missing:

CodeGen/CIRGenBuiltin.cpp:  case Builtin::BI__atomic_thread_fence:
CodeGen/CIRGenBuiltin.cpp-  case Builtin::BI__atomic_signal_fence:
CodeGen/CIRGenBuiltin.cpp:  case Builtin::BI__c11_atomic_thread_fence:
CodeGen/CIRGenBuiltin.cpp-  case Builtin::BI__c11_atomic_signal_fence:
CodeGen/CIRGenBuiltin.cpp:    llvm_unreachable("BI__atomic_thread_fence like NYI");

Activity

changed the title CIRGen support for `*thread_fence` CIRGen support for `*_atomic_{thread,signal}_fence` on Jan 9, 2025
elhewaty

elhewaty commented on Jan 9, 2025

@elhewaty
Member

I will give it a try, assign me please

bcardosolopes

bcardosolopes commented on Jan 10, 2025

@bcardosolopes
MemberAuthor

@elhewaty awesome, just did!

Rajveer100

Rajveer100 commented on Jan 11, 2025

@Rajveer100
Contributor

@bcardosolopes
Could you provide some context here with respect to the implementations?

From my understanding, this is similar to adding an operation in MLIR dialect, and then referencing with the cir:: namespace.

bcardosolopes

bcardosolopes commented on Jan 14, 2025

@bcardosolopes
MemberAuthor

@Rajveer100 sure, we probably want a new cir::FenceOp that encodes (a) the sync scope somehow (perhaps a attribute enum kind that covers single_thread and system) and (b) the atomic ordering. If the atomic ordering isn't constant, we don't want to early expand it during CIRGen, but only during LLVM Lowering. Lowering to LLVM should also be implemented as part of the PR that fixes this.

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

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Participants

@bcardosolopes@elhewaty@Rajveer100

Issue actions

    CIRGen support for `*_atomic_{thread,signal}_fence` · Issue #1274 · llvm/clangir