Skip to content

[OpenMPIRBuilder] Use BuildBuiltins for atomic operations #101966

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: users/meinersbur/llvm_buildbuiltins
Choose a base branch
from
Open

[OpenMPIRBuilder] Use BuildBuiltins for atomic operations #101966

wants to merge 1 commit into from

Conversation

Meinersbur
Copy link
Member

@Meinersbur Meinersbur commented Aug 5, 2024 •
edited
Loading

Use the code generation utilities from #134455 to emit atomic operations in the OpenMPIRBuilder.

The current logic that decides between using either an LLVM instruction or a libcall intrinsic does this decision based on the LLVM-IR type. For instance, a struct of two floats (represention a complex value) will call __atomic_* functions even though it is only 8 bytes and LLVM instructions could be uses as well. Fixed-sized libcall functions were also never used. This PR makes the decision which means to implement an atomic based on the number of bytes (and arguments compatibility) only, not the type. __atomic_* functions are not available on all platforms (e.g. Windows, see discussion at https://discourse.llvm.org/t/atomics-on-windows/84329), so we must not __atomic_* calls if not strictly needed.

The libcall functions have different signatures than the LLVM-IR instructions. The libcall functions generally pass values by pointers (because the data sizes are variable) whereas the LLVM-IR instructions pass-by-value (except the atomic pointers itself). The Builtin helpers abstract this away an only pass by-pointer. If the chosed method requires a pass-be-value, it it just loaded from memory. If the original data was an llvm::Value, a temporary alloca is created to have a memory location available for it. Mem2Reg/SROA will simply remove the helper alloca/store/load again, but the OpenMPIRBuilder does not have to care about target-specific details anymore.

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 5, 2024 •
edited
Loading

✅ With the latest revision this PR passed the C/C++ code formatter.

@nikic
Copy link
Contributor

nikic commented Aug 12, 2024

The TLI dependency in Clang CodeGen is a pretty substantial departure from how things currently work -- with the major implication that you can no longer reliably run codegen for a target for which the corresponding LLVM backend has not been compiled. Currently, codegen tests assume that this is not required and that they can be run without REQUIRES: *-registered-target.

@Meinersbur
Copy link
Member Author

TargetLibraryInfoImpl is not contained in the backends, but implemented entirely in TargetLibraryInfo.cpp.

TargetLowering is implemented in the backend, but is also optional. If not provided, the max size of atomic operations is assumed to be 16. I think that frontends having to resolve the ABI information independently would be worse.

@Meinersbur Meinersbur mentioned this pull request Sep 20, 2024
Merged
@Meinersbur Meinersbur mentioned this pull request Nov 4, 2024
Merged
@Meinersbur Meinersbur force-pushed the llvm_buildbuiltins branch from fd3b513 to 945b7c8 Compare March 3, 2025 15:05
@Meinersbur Meinersbur requested a review from NimishMishra March 3, 2025 15:05
@Meinersbur Meinersbur changed the title Add emitAtomicCompareExchangeBuiltin helper function [Transforms] Introduce BuildBuildins.h atomic helpers Mar 3, 2025
@Meinersbur Meinersbur marked this pull request as ready for review March 3, 2025 16:28
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. mlir:llvm mlir flang Flang issues not falling into any other category llvm:support mlir:openmp flang:openmp llvm:analysis llvm:transforms flang:semantics clang:openmp OpenMP related changes to Clang labels Mar 3, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 3, 2025 •
edited
Loading

@llvm/pr-subscribers-llvm-analysis
@llvm/pr-subscribers-mlir-openmp
@llvm/pr-subscribers-mlir-llvm
@llvm/pr-subscribers-flang-semantics
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-clang-codegen

Author: Michael Kruse (Meinersbur)

Changes

Introduce BuildBuildins.h atomic helpers and uses them in OpenMPIRBuilder

The idea is for it to contain code generation functions to be used by frontends akin to BuildLibcall.h. While BuildLibcall will only emit a specific libcall function, BuildBuiltin can choose between multiple implementations. Three builtins for emitting atomic access have been implemented:

  • atomic load (emitAtomicLoadBuiltin)
  • atomic store (emitAtomicStoreBuiltin)
  • atomic compare & exchange (emitAtomicCompareExchangeBuiltin)

The functions in this file are intended to handle the complexity of builtins so frontends do not need to care about the details. A major difference between the cases is that the IR instructions take values directly as an llvm::Value but the libcall functions almost always take pointers to those values. This abstraction passes everything as pointers. The caller is responsible to emit a temporary AllocaInst and store if it needs to pass an llvm::Value. Mem2Reg/SROA will easily remove any unnecessary store/load pairs. As a side-effect, thanks to opaque pointers, no explicit cast is ever needed.

This is meant to be a potential substitute for implementations in Clang's CGAtomic, LLVM's AtomicExpandPass, LLVM's FrontendAtomic, LLVM's OpenMPIRBuilder's handling of atomics, as well as lowering of atomics from MLIR and Flang (as well as any other frontend). Potentially also LLVM's LowerAtomicPass and LLVM's NVPTXAtomicLower. So there is a lot of redundancy.

Also see the discussion at https://discourse.llvm.org/t/rfc-refactoring-cgatomic-into-llvmfrontend/80168/6

Internal Details

In case of emitAtomicCompareExchangeBuiltin, it may emit either a cmpxchg instruction, multiple cmpxchg instructions to fulfil its constant argument requirement, a sized __atomic_compare_exchange_n libcall function call, or a generic __atomic_compare_exchange libcall function. Future enhancements may also do a fallback lowering using locks, necessary for platforms where no libcall equivalent is evailable (see https://discourse.llvm.org/t/atomics-on-windows/84329).

A lot of ABI information is represented redundantly by Clang as well as similar LLVM objects. Without access to Clang's ABI information, one must at least provide accesses to TargetLibraryInfo. Clang previously only created these objects when invoking the LLVM pass pipeline, but not emitting LLVM-IR. To make the ABI information available, these objects need to be created earlier.

The different implementations on atomic lowering do not agree on the details, for instanc lowering of libcall functions is done in Clang with the help of CodeGenModule::CreateRuntimeFunction, in LLVM with the help of TargetLibraryInfo, and once more in LLVM using the llvm::RTLIB utilities. Unfortunately they disagree on how to call libcall functions. For instance, Clang emits boolean return values as i1, but TargetLibraryInfo uses i8. TargetLibraryInfo adds function attributes to known libcall functions while CGAtomic only uses the same generic properties for all libcall functions. TargetLibraryInfo has list of libcall functions supported by a target but RTLIB does not. While RTLIB is more Machine-IR-centric, AtomicExpandPass still uses it and is happy to emit libcall functions that TargetLibraryInfo thinks are not supported. AtomicExpandPass can lower to sized __atomic_compare_exchange_n functions, CGAtomic does not. etc. Clang thinks fp80 "long double" has 12 bytes while for AtomicCompareExchange it is 10.

Patch is 3.13 MiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/101966.diff

19 Files Affected:

  • (modified) clang/lib/CodeGen/CGStmtOpenMP.cpp (+8-6)
  • (modified) clang/test/OpenMP/atomic_compare_codegen.cpp (+23272-12978)
  • (modified) flang/lib/Semantics/CMakeLists.txt (+43-22)
  • (modified) flang/test/Integration/OpenMP/atomic-capture-complex.f90 (+36-32)
  • (modified) llvm/include/llvm/Analysis/TargetLibraryInfo.def (+80)
  • (modified) llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h (+20-21)
  • (modified) llvm/include/llvm/Support/AtomicOrdering.h (+22)
  • (added) llvm/include/llvm/Transforms/Utils/BuildBuiltins.h (+283)
  • (modified) llvm/include/llvm/Transforms/Utils/BuildLibCalls.h (+47)
  • (modified) llvm/lib/Analysis/TargetLibraryInfo.cpp (+21)
  • (modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+238-194)
  • (added) llvm/lib/Transforms/Utils/BuildBuiltins.cpp (+826)
  • (modified) llvm/lib/Transforms/Utils/BuildLibCalls.cpp (+212)
  • (modified) llvm/lib/Transforms/Utils/CMakeLists.txt (+1)
  • (modified) llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml (+2-2)
  • (modified) llvm/unittests/Analysis/TargetLibraryInfoTest.cpp (+18)
  • (modified) llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp (+1153-376)
  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+11-3)
  • (modified) mlir/test/Target/LLVMIR/openmp-llvm.mlir (+272-201)
diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index e4d1db264aac9..a8ffe1edd1202 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -6740,16 +6740,18 @@ static void emitOMPAtomicCompareExpr(
               R->getType().isVolatileQualified()};
   }
 
+  llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
+      CGF.AllocaInsertPt->getParent(), CGF.AllocaInsertPt->getIterator());
   if (FailAO == llvm::AtomicOrdering::NotAtomic) {
     // fail clause was not mentioned on the
     // "#pragma omp atomic compare" construct.
-    CGF.Builder.restoreIP(OMPBuilder.createAtomicCompare(
-        CGF.Builder, XOpVal, VOpVal, ROpVal, EVal, DVal, AO, Op, IsXBinopExpr,
-        IsPostfixUpdate, IsFailOnly));
+    CGF.Builder.restoreIP(cantFail(OMPBuilder.createAtomicCompare(
+        CGF.Builder, AllocaIP, XOpVal, VOpVal, ROpVal, EVal, DVal, AO, Op,
+        IsXBinopExpr, IsPostfixUpdate, IsFailOnly)));
   } else
-    CGF.Builder.restoreIP(OMPBuilder.createAtomicCompare(
-        CGF.Builder, XOpVal, VOpVal, ROpVal, EVal, DVal, AO, Op, IsXBinopExpr,
-        IsPostfixUpdate, IsFailOnly, FailAO));
+    CGF.Builder.restoreIP(cantFail(OMPBuilder.createAtomicCompare(
+        CGF.Builder, AllocaIP, XOpVal, VOpVal, ROpVal, EVal, DVal, AO, Op,
+        IsXBinopExpr, IsPostfixUpdate, IsFailOnly, FailAO)));
 }
 
 static void emitOMPAtomicExpr(CodeGenFunction &CGF, OpenMPClauseKind Kind,
diff --git a/clang/test/OpenMP/atomic_compare_codegen.cpp b/clang/test/OpenMP/atomic_compare_codegen.cpp
index 03e5081a5c1d8..ff6e8fe3afea8 100644
--- a/clang/test/OpenMP/atomic_compare_codegen.cpp
+++ b/clang/test/OpenMP/atomic_compare_codegen.cpp
@@ -13903,6 +13903,726 @@ double fail_dxevd() {
 // CHECK-NEXT:    [[DX:%.*]] = alloca double, align 8
 // CHECK-NEXT:    [[DE:%.*]] = alloca double, align 8
 // CHECK-NEXT:    [[DD:%.*]] = alloca double, align 8
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR1:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR2:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR3:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR4:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR10:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR11:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR12:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR18:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR19:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR20:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR26:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR27:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR28:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR29:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR35:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR36:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR37:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR43:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR44:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR45:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR51:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR52:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR53:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR59:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR60:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR61:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR67:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR68:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR69:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR75:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR76:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR77:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR83:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR84:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR85:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR91:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR92:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR93:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR99:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR100:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR101:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR107:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR108:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR109:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR115:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR116:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR117:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR123:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR124:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR125:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR131:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR132:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR133:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR139:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR140:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR141:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR147:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR148:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR149:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR155:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR156:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR157:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR163:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR164:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR165:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR171:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR172:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR173:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR179:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR180:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR181:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR187:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR188:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR189:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR195:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR196:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR197:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR203:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR204:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR205:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR211:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR212:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR213:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR219:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR220:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR221:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR227:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR228:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR229:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR235:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR236:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR237:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR243:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR244:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR245:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR251:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR252:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR253:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR259:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR260:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR261:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR267:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR268:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR269:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR275:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR276:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR277:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR283:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR284:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR285:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR291:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR292:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR293:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR299:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR300:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR301:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR307:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR308:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR309:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR315:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR316:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR317:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR323:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR324:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR325:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR331:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_DESIRED_PTR332:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[CX_ATOMIC_EXPECTED_PTR333:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR339:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR340:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR341:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR347:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR348:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR349:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR355:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR356:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR357:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR363:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_DESIRED_PTR364:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[UCX_ATOMIC_EXPECTED_PTR365:%.*]] = alloca i8, align 1
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR371:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR372:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR373:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR374:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR380:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR381:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR382:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR388:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR389:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR390:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR396:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR397:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR398:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR399:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR405:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR406:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR407:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR413:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR414:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR415:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR421:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR422:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR423:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR429:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR430:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR431:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR437:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR438:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR439:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR445:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR446:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR447:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR453:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR454:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR455:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR461:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR462:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR463:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR469:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR470:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR471:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR477:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR478:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR479:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR485:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR486:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR487:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR493:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR494:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR495:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR501:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR502:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR503:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR509:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR510:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR511:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR517:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR518:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR519:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR525:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR526:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR527:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR533:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR534:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR535:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR541:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR542:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR543:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR549:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR550:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR551:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR557:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR558:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR559:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR565:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR566:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR567:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR573:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR574:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR575:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR581:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR582:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR583:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR589:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR590:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR591:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR597:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR598:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR599:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR605:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_DESIRED_PTR606:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[USX_ATOMIC_EXPECTED_PTR607:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR613:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR614:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR615:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR621:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR622:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR623:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR629:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_DESIRED_PTR630:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR631:%.*]] = alloca i16, align 2
+// CHECK-NEXT:    [[SX_ATOMIC_EXPECTED_PTR637:%.*]] ...
[truncated]

@jyknight jyknight requested review from arsenm and jyknight March 3, 2025 19:57
@nikic nikic changed the title [Transforms] Introduce BuildBuildins.h atomic helpers [Transforms] Introduce BuildBuiltins.h atomic helpers Mar 3, 2025
@NimishMishra NimishMishra requested a review from tblah March 17, 2025 09:01
tblah
tblah reviewed Mar 17, 2025
View reviewed changes
Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

The idea behind this sounds good to me, and this patch improves flang's test results on the fujitsu testsutie, shows no regressions in the gfortran test suite, and fixes the important remaining flang issues from omp atomic. Atomic support is one of the most lagging areas in Flang's OpenMP implementation so I would be very happy to see this land.

But I think we need agreement from clang folks for this. The result of the RFC looks inconclusive and @nikic has raised concerns on this patch.

@NimishMishra NimishMishra mentioned this pull request Mar 19, 2025
Closed
This was referenced Mar 21, 2025
Merged
Merged
@Meinersbur Meinersbur mentioned this pull request Apr 4, 2025
Open
@Meinersbur Meinersbur force-pushed the llvm_buildbuiltins branch from c5ce3bc to 1830ed2 Compare April 7, 2025 12:54
@Meinersbur Meinersbur changed the base branch from main to users/meinersbur/llvm_buildbuiltins April 7, 2025 12:54
@Meinersbur Meinersbur changed the title [Transforms] Introduce BuildBuiltins.h atomic helpers [OpenMPIRBuilder] Use BuildBuildins for atomic operations Apr 7, 2025
@Meinersbur
Copy link
Member Author

I split this PR into two parts:

@Meinersbur Meinersbur changed the title [OpenMPIRBuilder] Use BuildBuildins for atomic operations [OpenMPIRBuilder] Use BuildBuiltins for atomic operations Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tblah tblah tblah left review comments

@NimishMishra NimishMishra Awaiting requested review from NimishMishra

@arsenm arsenm Awaiting requested review from arsenm

@jyknight jyknight Awaiting requested review from jyknight

Assignees
No one assigned
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category flang:openmp flang:semantics flang Flang issues not falling into any other category llvm:analysis llvm:support llvm:transforms mlir:llvm mlir:openmp mlir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Meinersbur @nikic @llvmbot @tblah