[OpenMP] atomic compare fail : Codegen support #75709

SunilKuravinakop · 2023-12-16T18:55:35Z

This is a continuation of https://reviews.llvm.org/D123235 ([OpenMP] atomic compare fail : Parser & AST support). In this branch Support for codegen support for atomic compare fail is being added.

Changes to be committed: modified: clang/lib/CodeGen/CGStmtOpenMP.cpp

llvmbot · 2023-12-16T18:56:02Z

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-clang

Author: None (SunilKuravinakop)

Changes

This is a continuation of https://reviews.llvm.org/D123235 ([OpenMP] atomic compare fail : Parser & AST support). In this branch Support for codegen support for atomic compare fail is being added.

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

1 Files Affected:

(modified) clang/lib/CodeGen/CGStmtOpenMP.cpp (+47-4)

diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index ed426098ac6915..e2caa0d7742126 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -6508,10 +6508,6 @@ static void emitOMPAtomicExpr(CodeGenFunction &CGF, OpenMPClauseKind Kind,
                              IsPostfixUpdate, IsFailOnly, Loc);
     break;
   }
-  case OMPC_fail: {
-    //TODO
-    break;
-  }
   default:
     llvm_unreachable("Clause is not allowed in 'omp atomic'.");
   }
@@ -6519,6 +6515,8 @@ static void emitOMPAtomicExpr(CodeGenFunction &CGF, OpenMPClauseKind Kind,
 
 void CodeGenFunction::EmitOMPAtomicDirective(const OMPAtomicDirective &S) {
   llvm::AtomicOrdering AO = llvm::AtomicOrdering::Monotonic;
+  // Fail Memory Clause Ordering.
+  llvm::AtomicOrdering FO = llvm::AtomicOrdering::Monotonic;
   bool MemOrderingSpecified = false;
   if (S.getSingleClause<OMPSeqCstClause>()) {
     AO = llvm::AtomicOrdering::SequentiallyConsistent;
@@ -6572,6 +6570,51 @@ void CodeGenFunction::EmitOMPAtomicDirective(const OMPAtomicDirective &S) {
     }
   }
 
+  if (KindsEncountered.contains(OMPC_compare) &&
+      KindsEncountered.contains(OMPC_fail)) {
+    Kind = OMPC_compare;
+    const OMPFailClause *fC = S.getSingleClause<OMPFailClause>();
+    if (fC) {
+      OpenMPClauseKind fP = fC->getFailParameter();
+      if (fP == llvm::omp::OMPC_relaxed)
+        FO = llvm::AtomicOrdering::Monotonic;
+      else if (fP == llvm::omp::OMPC_acquire)
+        FO = llvm::AtomicOrdering::Acquire;
+      else if (fP == llvm::omp::OMPC_seq_cst)
+        FO = llvm::AtomicOrdering::SequentiallyConsistent;
+    }
+
+    // Logic for 2 memory order clauses in the atomic directive.
+    // e.g. #pragma omp atomic compare capture release fail(seq_cst)
+    //      if(x > e) { x = j; } else { k = x; }
+    // To provide the Memory Order clause in atomic directive
+    // there are 2 instructions in LLVM IR atomicrmw & cmpxchgl.
+    // 1) atomicrmw can use only 1 memory order clause and can contain the
+    //    operator in if condition.
+    // 2) cmpxchgl can use 2 memory order clauses : Success memory order clause
+    //    & fail parameter memory clause. However, cmpxchgl uses only equality
+    //    operator.
+    // We need to change atomicrmw to contain the fail parameter clause or add
+    // a new instruction in LLVM IR. Changes in LLVM IR need to be done
+    // seperately and at present we will use the logic of using the more strict
+    // memory order clause of success or fail memory order clauses for the
+    // atomicrmw. The following logic takes care of this change in the memory
+    // order clause.
+    if (AO == llvm::AtomicOrdering::Monotonic)
+      AO = FO;
+    else if (FO == llvm::AtomicOrdering::Monotonic)
+      AO = AO;
+    else if (AO == llvm::AtomicOrdering::SequentiallyConsistent ||
+             FO == llvm::AtomicOrdering::SequentiallyConsistent)
+      AO = llvm::AtomicOrdering::SequentiallyConsistent;
+    else if (AO == llvm::AtomicOrdering::Acquire)
+      AO = llvm::AtomicOrdering::Acquire;
+    else if (AO == llvm::AtomicOrdering::Release)
+      AO = llvm::AtomicOrdering::AcquireRelease;
+    else if (AO == llvm::AtomicOrdering::AcquireRelease)
+      AO = llvm::AtomicOrdering::AcquireRelease;
+  }
+
   LexicalScope Scope(*this, S.getSourceRange());
   EmitStopPoint(S.getAssociatedStmt());
   emitOMPAtomicExpr(*this, Kind, AO, S.isPostfixUpdate(), S.getX(), S.getV(),

llvmbot · 2023-12-16T18:56:02Z

@llvm/pr-subscribers-clang-codegen

Author: None (SunilKuravinakop)

Changes

This is a continuation of https://reviews.llvm.org/D123235 ([OpenMP] atomic compare fail : Parser & AST support). In this branch Support for codegen support for atomic compare fail is being added.

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

1 Files Affected:

(modified) clang/lib/CodeGen/CGStmtOpenMP.cpp (+47-4)

diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index ed426098ac6915..e2caa0d7742126 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -6508,10 +6508,6 @@ static void emitOMPAtomicExpr(CodeGenFunction &CGF, OpenMPClauseKind Kind,
                              IsPostfixUpdate, IsFailOnly, Loc);
     break;
   }
-  case OMPC_fail: {
-    //TODO
-    break;
-  }
   default:
     llvm_unreachable("Clause is not allowed in 'omp atomic'.");
   }
@@ -6519,6 +6515,8 @@ static void emitOMPAtomicExpr(CodeGenFunction &CGF, OpenMPClauseKind Kind,
 
 void CodeGenFunction::EmitOMPAtomicDirective(const OMPAtomicDirective &S) {
   llvm::AtomicOrdering AO = llvm::AtomicOrdering::Monotonic;
+  // Fail Memory Clause Ordering.
+  llvm::AtomicOrdering FO = llvm::AtomicOrdering::Monotonic;
   bool MemOrderingSpecified = false;
   if (S.getSingleClause<OMPSeqCstClause>()) {
     AO = llvm::AtomicOrdering::SequentiallyConsistent;
@@ -6572,6 +6570,51 @@ void CodeGenFunction::EmitOMPAtomicDirective(const OMPAtomicDirective &S) {
     }
   }
 
+  if (KindsEncountered.contains(OMPC_compare) &&
+      KindsEncountered.contains(OMPC_fail)) {
+    Kind = OMPC_compare;
+    const OMPFailClause *fC = S.getSingleClause<OMPFailClause>();
+    if (fC) {
+      OpenMPClauseKind fP = fC->getFailParameter();
+      if (fP == llvm::omp::OMPC_relaxed)
+        FO = llvm::AtomicOrdering::Monotonic;
+      else if (fP == llvm::omp::OMPC_acquire)
+        FO = llvm::AtomicOrdering::Acquire;
+      else if (fP == llvm::omp::OMPC_seq_cst)
+        FO = llvm::AtomicOrdering::SequentiallyConsistent;
+    }
+
+    // Logic for 2 memory order clauses in the atomic directive.
+    // e.g. #pragma omp atomic compare capture release fail(seq_cst)
+    //      if(x > e) { x = j; } else { k = x; }
+    // To provide the Memory Order clause in atomic directive
+    // there are 2 instructions in LLVM IR atomicrmw & cmpxchgl.
+    // 1) atomicrmw can use only 1 memory order clause and can contain the
+    //    operator in if condition.
+    // 2) cmpxchgl can use 2 memory order clauses : Success memory order clause
+    //    & fail parameter memory clause. However, cmpxchgl uses only equality
+    //    operator.
+    // We need to change atomicrmw to contain the fail parameter clause or add
+    // a new instruction in LLVM IR. Changes in LLVM IR need to be done
+    // seperately and at present we will use the logic of using the more strict
+    // memory order clause of success or fail memory order clauses for the
+    // atomicrmw. The following logic takes care of this change in the memory
+    // order clause.
+    if (AO == llvm::AtomicOrdering::Monotonic)
+      AO = FO;
+    else if (FO == llvm::AtomicOrdering::Monotonic)
+      AO = AO;
+    else if (AO == llvm::AtomicOrdering::SequentiallyConsistent ||
+             FO == llvm::AtomicOrdering::SequentiallyConsistent)
+      AO = llvm::AtomicOrdering::SequentiallyConsistent;
+    else if (AO == llvm::AtomicOrdering::Acquire)
+      AO = llvm::AtomicOrdering::Acquire;
+    else if (AO == llvm::AtomicOrdering::Release)
+      AO = llvm::AtomicOrdering::AcquireRelease;
+    else if (AO == llvm::AtomicOrdering::AcquireRelease)
+      AO = llvm::AtomicOrdering::AcquireRelease;
+  }
+
   LexicalScope Scope(*this, S.getSourceRange());
   EmitStopPoint(S.getAssociatedStmt());
   emitOMPAtomicExpr(*this, Kind, AO, S.isPostfixUpdate(), S.getX(), S.getV(),

{if(x == e) {x = v;} else {d = x;}} results in generation of cmpxchg in the LLVM IR, the memory order clause in "fail" clause is now being added. In case of other operators ">" & "<" e.g. dx = dx > de ? de : dx; result in atomicrmw in the LLVM IR. "atomicrmw" can have only one memory order clause. Currently, the memory order clause is ignored. 2) Test cases for codegen 3) In SemaOpenMP.cpp, added EncounteredAtomicKinds.contains(OMPC_compare) instead of AtomicKind == OMPC_compare. This was to accomodate where AtomicKind is OMPC_capture. Changes to be committed: modified: clang/lib/CodeGen/CGStmtOpenMP.cpp modified: clang/lib/Sema/SemaOpenMP.cpp modified: clang/test/OpenMP/atomic_compare_codegen.cpp modified: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h modified: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

alexey-bataev · 2023-12-20T13:37:45Z

clang/lib/CodeGen/CGStmtOpenMP.cpp

+  if (KindsEncountered.contains(OMPC_compare) &&
+      KindsEncountered.contains(OMPC_fail)) {
+    Kind = OMPC_compare;
+    const OMPFailClause *fC = S.getSingleClause<OMPFailClause>();


Suggested change

const OMPFailClause *fC = S.getSingleClause<OMPFailClause>();

const auto *FC = S.getSingleClause<OMPFailClause>();

alexey-bataev · 2023-12-20T13:38:40Z

clang/lib/CodeGen/CGStmtOpenMP.cpp

+    Kind = OMPC_compare;
+    const OMPFailClause *fC = S.getSingleClause<OMPFailClause>();
+    if (fC) {
+      OpenMPClauseKind fP = fC->getFailParameter();


Suggested change

OpenMPClauseKind fP = fC->getFailParameter();

OpenMPClauseKind FP = fC->getFailParameter();

ddpagan

Aside from Alexey's comments, LGTM.

Changed the variable names of "fail" clause and the parameter to this fail clause more self-explanatory.

Sunil Kuravinakop added 2 commits December 16, 2023 11:43

Adding parameter to fail clause (i.e. memory order clause) for codegen.

fe931d6

Changes to be committed: modified: clang/lib/CodeGen/CGStmtOpenMP.cpp

Merge branch 'main' into codegen_atomic_compare_fail

1031eae

llvmbot added clang Clang issues not falling into any other category clang:codegen labels Dec 16, 2023

SunilKuravinakop marked this pull request as draft December 17, 2023 11:59

Sunil Kuravinakop and others added 3 commits December 19, 2023 23:49

Merge branch 'main' into codegen_atomic_compare_fail

f93bc94

Merge branch 'llvm:main' into codegen_atomic_compare_fail

9514881

SunilKuravinakop marked this pull request as ready for review December 20, 2023 08:57

llvmbot added clang:frontend Language frontend issues, e.g. anything involving "Sema" flang:openmp clang:openmp OpenMP related changes to Clang labels Dec 20, 2023

sandeepkosuri requested review from alexey-bataev, ddpagan, jhuber6 and sandeepkosuri December 20, 2023 11:15

alexey-bataev reviewed Dec 20, 2023

View reviewed changes

ddpagan reviewed Dec 20, 2023

View reviewed changes

Changes in name format from Alexey.

44dbdb0

Changed the variable names of "fail" clause and the parameter to this fail clause more self-explanatory.

alexey-bataev approved these changes Dec 20, 2023

View reviewed changes

Sunil Kuravinakop added 4 commits December 21, 2023 02:38

Merge branch 'main' into codegen_atomic_compare_fail

c1b3508

Merge branch 'main' into codegen_atomic_compare_fail

c806dfb

Merge branch 'main' into codegen_atomic_compare_fail

4de72a4

Merge branch 'main' into codegen_atomic_compare_fail

c317528

sandeepkosuri merged commit 49ee8b5 into llvm:main Jan 2, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[OpenMP] atomic compare fail : Codegen support #75709

[OpenMP] atomic compare fail : Codegen support #75709

SunilKuravinakop commented Dec 16, 2023

llvmbot commented Dec 16, 2023 •

edited

llvmbot commented Dec 16, 2023

alexey-bataev Dec 20, 2023

alexey-bataev Dec 20, 2023

ddpagan left a comment

	const OMPFailClause *fC = S.getSingleClause<OMPFailClause>();
	const auto *FC = S.getSingleClause<OMPFailClause>();

	OpenMPClauseKind fP = fC->getFailParameter();
	OpenMPClauseKind FP = fC->getFailParameter();

[OpenMP] atomic compare fail : Codegen support #75709

[OpenMP] atomic compare fail : Codegen support #75709

Conversation

SunilKuravinakop commented Dec 16, 2023

llvmbot commented Dec 16, 2023 • edited

llvmbot commented Dec 16, 2023

alexey-bataev Dec 20, 2023

Choose a reason for hiding this comment

alexey-bataev Dec 20, 2023

Choose a reason for hiding this comment

ddpagan left a comment

Choose a reason for hiding this comment

llvmbot commented Dec 16, 2023 •

edited