Skip to content

[OpenMP] Correctly code-gen default atomic mem order #97663

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.

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

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Jul 4, 2024

Summary:
The parsing for this was implemented, but we never hooked up the default
value to the result of this clause. This patch adds the support by
making it default to the requires directive.

Summary:
The parsing for this was implemented, but we never hooked up the default
value to the result of this clause. This patch adds the support by
making it default to the requires directive.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. clang:openmp OpenMP related changes to Clang labels Jul 4, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 4, 2024

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Joseph Huber (jhuber6)

Changes

Summary:
The parsing for this was implemented, but we never hooked up the default
value to the result of this clause. This patch adds the support by
making it default to the requires directive.


Full diff: https://github.com/llvm/llvm-project/pull/97663.diff

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGStmtOpenMP.cpp (+1-1)
  • (added) clang/test/OpenMP/requires_default_atomic_mem_order.cpp (+46)
diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 76ff8f5b234da..4d05322951d0a 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -6555,7 +6555,7 @@ static void emitOMPAtomicExpr(CodeGenFunction &CGF, OpenMPClauseKind Kind,
 }
 
 void CodeGenFunction::EmitOMPAtomicDirective(const OMPAtomicDirective &S) {
-  llvm::AtomicOrdering AO = llvm::AtomicOrdering::Monotonic;
+  llvm::AtomicOrdering AO = CGM.getOpenMPRuntime().getDefaultMemoryOrdering();
   // Fail Memory Clause Ordering.
   llvm::AtomicOrdering FailAO = llvm::AtomicOrdering::NotAtomic;
   bool MemOrderingSpecified = false;
diff --git a/clang/test/OpenMP/requires_default_atomic_mem_order.cpp b/clang/test/OpenMP/requires_default_atomic_mem_order.cpp
new file mode 100644
index 0000000000000..90d2db4eac20c
--- /dev/null
+++ b/clang/test/OpenMP/requires_default_atomic_mem_order.cpp
@@ -0,0 +1,46 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
+// RUN: %clang_cc1 -emit-llvm -fopenmp -triple=x86_64-unknown-linux-gnu \
+// RUN:   -DORDERING=seq_cst -o - %s \
+// RUN: | FileCheck %s --check-prefix=SEQ_CST
+// RUN: %clang_cc1 -emit-llvm -fopenmp -triple=x86_64-unknown-linux-gnu \
+// RUN:   -DORDERING=acq_rel -o - %s \
+// RUN: | FileCheck %s --check-prefix=ACQ_REL
+// RUN: %clang_cc1 -emit-llvm -fopenmp -triple=x86_64-unknown-linux-gnu \
+// RUN:   -DORDERING=relaxed -o - %s \
+// RUN: | FileCheck %s --check-prefix=RELAXED
+
+#pragma omp requires atomic_default_mem_order(ORDERING)
+
+// SEQ_CST-LABEL: define dso_local void @_Z3fooPi(
+// SEQ_CST-SAME: ptr noundef [[X:%.*]]) #[[ATTR0:[0-9]+]] {
+// SEQ_CST-NEXT:  [[ENTRY:.*:]]
+// SEQ_CST-NEXT:    [[X_ADDR:%.*]] = alloca ptr, align 8
+// SEQ_CST-NEXT:    store ptr [[X]], ptr [[X_ADDR]], align 8
+// SEQ_CST-NEXT:    [[TMP0:%.*]] = load ptr, ptr [[X_ADDR]], align 8
+// SEQ_CST-NEXT:    [[TMP1:%.*]] = atomicrmw add ptr [[TMP0]], i32 1 seq_cst, align 4
+// SEQ_CST-NEXT:    call void @__kmpc_flush(ptr @[[GLOB1:[0-9]+]])
+// SEQ_CST-NEXT:    ret void
+//
+// ACQ_REL-LABEL: define dso_local void @_Z3fooPi(
+// ACQ_REL-SAME: ptr noundef [[X:%.*]]) #[[ATTR0:[0-9]+]] {
+// ACQ_REL-NEXT:  [[ENTRY:.*:]]
+// ACQ_REL-NEXT:    [[X_ADDR:%.*]] = alloca ptr, align 8
+// ACQ_REL-NEXT:    store ptr [[X]], ptr [[X_ADDR]], align 8
+// ACQ_REL-NEXT:    [[TMP0:%.*]] = load ptr, ptr [[X_ADDR]], align 8
+// ACQ_REL-NEXT:    [[TMP1:%.*]] = atomicrmw add ptr [[TMP0]], i32 1 release, align 4
+// ACQ_REL-NEXT:    call void @__kmpc_flush(ptr @[[GLOB1:[0-9]+]])
+// ACQ_REL-NEXT:    ret void
+//
+// RELAXED-LABEL: define dso_local void @_Z3fooPi(
+// RELAXED-SAME: ptr noundef [[X:%.*]]) #[[ATTR0:[0-9]+]] {
+// RELAXED-NEXT:  [[ENTRY:.*:]]
+// RELAXED-NEXT:    [[X_ADDR:%.*]] = alloca ptr, align 8
+// RELAXED-NEXT:    store ptr [[X]], ptr [[X_ADDR]], align 8
+// RELAXED-NEXT:    [[TMP0:%.*]] = load ptr, ptr [[X_ADDR]], align 8
+// RELAXED-NEXT:    [[TMP1:%.*]] = atomicrmw add ptr [[TMP0]], i32 1 monotonic, align 4
+// RELAXED-NEXT:    ret void
+//
+void foo(int *x) {
+  #pragma omp atomic update
+    *x = *x + 1;
+}

Copy link
Member

@jdoerfert jdoerfert left a comment

Choose a reason for hiding this comment

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

LG

@jdoerfert jdoerfert merged commit 5ef4e6d into llvm:main Jul 8, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants