search

llvm / clangir

A new (MLIR based) high-level IR for clang.
https://clangir.org
Other
308 stars 84 forks source link

[CIR][CIRGen] Builtins: add __sync_fetch_and_add #631

Closed gitoleg closed 1 month ago

gitoleg commented 1 month ago

This PR adds support for atomic __sync_fetch_and_add.

Basically it's a copy-pasta from the original codegen. The only thing that I doubt about is what exact operation I need to create in CIR. The first approach I used was to create AtomicRMW operation in CIR. But as far as I see I can use the existing AtomicFetch instead. Is it correct? or it's better to add a new op here?

bcardosolopes commented 1 month ago

But as far as I see I can use the existing AtomicFetch instead. Is it correct?

Yep!

gitoleg commented 1 month ago

@bcardosolopes done

Can you also please add more tests like the ones for __sync_fetch_and_add in clang/test/CodeGen/Atomics.c?

I added more tests for different types . Or .. You meant we need to create another file for some reasons?

bcardosolopes commented 1 month ago

I added more tests for different types . Or .. You meant we need to create another file for some reasons?

This is fine, thanks!

gitoleg commented 1 month ago

This is still missing checks for the LLVM counterparts for the new ones you added, I'd like to make sure we are emitting the same code as OG codegen

@bcardosolopes updated!