[OPENMP]Fix PR45854: prevent code movement out of the critical region.

[ABataev]

Summary

Need to emit memory barriers upon enter/exit to/from critical region to
prevent code movement.

[mikerice]

Is it possible to have the same problem with other directives, like 'omp single' or 'omp master'? I haven't seen any test fail for those yet though.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

357

instructions

380

instructions

[ABataev]

In D81192#2074677, @mikerice wrote:

Is it possible to have the same problem with other directives, like 'omp single' or 'omp master'? I haven't seen any test fail for those yet though.

I rather doubt. The main problem with the critical directive that load/stores should not be moved out of the region (they all should be executed in the critical region context, just like atomics). For single and master it should be fine.

[jdoerfert]

I doubt this is the right fix. I'll take a closer look.

[ABataev]

In D81192#2074732, @jdoerfert wrote:

I doubt this is the right fix. I'll take a closer look.

I believe, the issue is caused by the new attributes for kmpc_critical/kmpc_end_critical functions. They are marked as InaccessibleMemOrArgMemOnly functions. Previously they did not have this attribute and optimizer did not move the code across these function calls.

[ABataev]

Rebase and fixes

[jdoerfert]

In D81192#2074762, @ABataev wrote:

In D81192#2074732, @jdoerfert wrote:

I doubt this is the right fix. I'll take a closer look.

I believe, the issue is caused by the new attributes for kmpc_critical/kmpc_end_critical functions. They are marked as InaccessibleMemOrArgMemOnly functions. Previously they did not have this attribute and optimizer did not move the code across these function calls.

I would argue it is exposed not caused by the attribute ;)

See PR46210

[ABataev]

In D81192#2074878, @jdoerfert wrote:

In D81192#2074762, @ABataev wrote:

In D81192#2074732, @jdoerfert wrote:

I doubt this is the right fix. I'll take a closer look.

I believe, the issue is caused by the new attributes for kmpc_critical/kmpc_end_critical functions. They are marked as InaccessibleMemOrArgMemOnly functions. Previously they did not have this attribute and optimizer did not move the code across these function calls.

I would argue it is exposed not caused by the attribute ;)

See PR46210

Agree, the attribute exposes the issue.

clang/lib/CodeGen/CGOpenMPRuntime.cpp

 namespace {
 /// Common pre(post)-action for different OpenMP constructs.
 class CommonActionTy final : public PrePostActionTy {
   llvm::FunctionCallee EnterCallee;
   ArrayRef<llvm::Value *> EnterArgs;
   llvm::FunctionCallee ExitCallee;
   ArrayRef<llvm::Value *> ExitArgs;
   bool Conditional;
+  bool WithMemBarrier = false;
   llvm::BasicBlock *ContBlock = nullptr;
 
 public:
   CommonActionTy(llvm::FunctionCallee EnterCallee,
                  ArrayRef<llvm::Value *> EnterArgs,
                  llvm::FunctionCallee ExitCallee,
-                 ArrayRef<llvm::Value *> ExitArgs, bool Conditional = false)
+                 ArrayRef<llvm::Value *> ExitArgs, bool Conditional = false,
+                 bool WithMemBarrier = false)
       : EnterCallee(EnterCallee), EnterArgs(EnterArgs), ExitCallee(ExitCallee),
-        ExitArgs(ExitArgs), Conditional(Conditional) {}
+        ExitArgs(ExitArgs), Conditional(Conditional),
+        WithMemBarrier(WithMemBarrier) {}
   void Enter(CodeGenFunction &CGF) override {
     llvm::Value *EnterRes = CGF.EmitRuntimeCall(EnterCallee, EnterArgs);
     if (Conditional) {
       llvm::Value *CallBool = CGF.Builder.CreateIsNotNull(EnterRes);
       auto *ThenBlock = CGF.createBasicBlock("omp_if.then");
       ContBlock = CGF.createBasicBlock("omp_if.end");
       // Generate the branch (If-stmt)
       CGF.Builder.CreateCondBr(CallBool, ThenBlock, ContBlock);
       CGF.EmitBlock(ThenBlock);
     }
+    if (WithMemBarrier)
+      CGF.Builder.CreateFence(llvm::AtomicOrdering::Acquire);
   }
   void Done(CodeGenFunction &CGF) {
     // Emit the rest of blocks/branches
     CGF.EmitBranch(ContBlock);
     CGF.EmitBlock(ContBlock, true);
   }
   void Exit(CodeGenFunction &CGF) override {
+    if (WithMemBarrier)
+      CGF.Builder.CreateFence(llvm::AtomicOrdering::Release);
     CGF.EmitRuntimeCall(ExitCallee, ExitArgs);
   }
 };
 } // anonymous namespace
 
 void CGOpenMPRuntime::emitCriticalRegion(CodeGenFunction &CGF,
                                          StringRef CriticalName,
                                          const RegionCodeGenTy &CriticalOpGen,
void CGOpenMPRuntime::emitCriticalRegion(CodeGenFunction &CGF,
   }
   CommonActionTy Action(
       llvm::OpenMPIRBuilder::getOrCreateRuntimeFunction(
           CGM.getModule(),
           Hint ? OMPRTL___kmpc_critical_with_hint : OMPRTL___kmpc_critical),
       EnterArgs,
       llvm::OpenMPIRBuilder::getOrCreateRuntimeFunction(
           CGM.getModule(), OMPRTL___kmpc_end_critical),
-      Args);
+      Args, /*Conditional=*/false, /*WithMemBarrier=*/true);
   CriticalOpGen.setAction(Action);
   emitInlinedDirective(CGF, OMPD_critical, CriticalOpGen);
 }
 
 void CGOpenMPRuntime::emitMasterRegion(CodeGenFunction &CGF,
                                        const RegionCodeGenTy &MasterOpGen,
                                        SourceLocation Loc) {
   if (!CGF.HaveInsertPoint())

clang/test/OpenMP/critical_codegen.cpp

 void foo() {}
 
 // ALL-LABEL: @main
 // TERM_DEBUG-LABEL: @main
 int main() {
   // ALL:       [[A_ADDR:%.+]] = alloca i8
   char a;
 
-// ALL:             [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
-// ALL:             call {{.*}}void @__kmpc_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[UNNAMED_LOCK]])
-// ALL-NEXT:        store i8 2, i8* [[A_ADDR]]
-// ALL-NEXT:        call {{.*}}void @__kmpc_end_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[UNNAMED_LOCK]])
+// ALL:       [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
+// ALL:       call {{.*}}void @__kmpc_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[UNNAMED_LOCK]])
+// ALL-NEXT:  fence acquire
+// ALL-NEXT:  store i8 2, i8* [[A_ADDR]]
+// ALL-NEXT:  fence release
+// ALL-NEXT:  call {{.*}}void @__kmpc_end_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[UNNAMED_LOCK]])
 #pragma omp critical
   a = 2;
 // IRBUILDER:       [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
-// ALL:             call {{.*}}void @__kmpc_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[THE_NAME_LOCK]])
-// IRBUILDER-NEXT:  call {{.*}}void [[FOO]]()
-// NORMAL-NEXT:     invoke {{.*}}void [[FOO]]()
-// ALL:             call {{.*}}void @__kmpc_end_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[THE_NAME_LOCK]])
+// ALL:             call {{.*}}void @__kmpc_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[THE_NAME_LOCK]])
+// ALL-NEXT:        fence acquire
+// IRBUILDER-NEXT:  call {{.*}}void [[FOO]]()
+// NORMAL-NEXT:     invoke {{.*}}void [[FOO]]()
+// ALL:             fence release
+// ALL-NEXT:        call {{.*}}void @__kmpc_end_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[THE_NAME_LOCK]])
 #pragma omp critical(the_name)
   foo();
-// IRBUILDER:       [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
-// ALL:             call {{.*}}void @__kmpc_critical_with_hint([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[THE_NAME_LOCK1]], i{{64|32}} 23)
-// IRBUILDER-NEXT:  call {{.*}}void [[FOO]]()
-// NORMAL-NEXT:     invoke {{.*}}void [[FOO]]()
-// ALL:             call {{.*}}void @__kmpc_end_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[THE_NAME_LOCK1]])
+// IRBUILDER:       [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
+// ALL:             call {{.*}}void @__kmpc_critical_with_hint([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[THE_NAME_LOCK1]], i{{64|32}} 23)
+// ALL-NEXT:        fence acquire
+// IRBUILDER-NEXT:  call {{.*}}void [[FOO]]()
+// NORMAL-NEXT:     invoke {{.*}}void [[FOO]]()
+// ALL:             fence release
+// ALL-NEXT:        call {{.*}}void @__kmpc_end_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[THE_NAME_LOCK1]])
 #pragma omp critical(the_name1) hint(23)
   foo();
-  // IRBUILDER:       [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
+  // IRBUILDER: [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
   // ALL:       call {{.*}}void @__kmpc_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[THE_NAME_LOCK]])
+  // ALL-NEXT:  fence acquire
   // ALL:       br label
   // ALL-NOT:   call {{.*}}void @__kmpc_end_critical(
   // ALL:       br label
   // ALL-NOT:   call {{.*}}void @__kmpc_end_critical(
   // NORMAL:       br label
   if (a)
 #pragma omp critical(the_name)
     while (1)

 void critical_ref(S &s) {
   // ALL: [[S_ADDR:%.+]] = alloca %struct.S*,
   // ALL: [[S_REF:%.+]] = load %struct.S*, %struct.S** [[S_ADDR]],
   // ALL: [[S_A_REF:%.+]] = getelementptr inbounds %struct.S, %struct.S* [[S_REF]], i32 0, i32 0
   ++s.a;
   // NORMAL: [[S_REF:%.+]] = load %struct.S*, %struct.S** [[S_ADDR]],
   // NORMAL: store %struct.S* [[S_REF]], %struct.S** [[S_ADDR:%.+]],
   // ALL: call void @__kmpc_critical(
+  // ALL: fence acquire
 #pragma omp critical
   // ALL: [[S_REF:%.+]] = load %struct.S*, %struct.S** [[S_ADDR]],
   // ALL: [[S_A_REF:%.+]] = getelementptr inbounds %struct.S, %struct.S* [[S_REF]], i32 0, i32 0
   ++s.a;
+  // ALL: fence release
   // ALL: call void @__kmpc_end_critical(
 }
 
 // ALL-LABEL:      parallel_critical
 // TERM_DEBUG-LABEL: parallel_critical
 void parallel_critical() {
 #pragma omp parallel
 #pragma omp critical
   // TERM_DEBUG-NOT: __kmpc_global_thread_num
   // TERM_DEBUG:     call void @__kmpc_critical({{.+}}), !dbg [[DBG_LOC_START:![0-9]+]]
+  // TERM_DEBUG:     fence acquire
   // TERM_DEBUG:     invoke void {{.*}}foo{{.*}}()
   // TERM_DEBUG:     unwind label %[[TERM_LPAD:.+]],
   // TERM_DEBUG-NOT: __kmpc_global_thread_num
+  // TERM_DEBUG:     fence release
   // TERM_DEBUG:     call void @__kmpc_end_critical({{.+}}), !dbg [[DBG_LOC_END:![0-9]+]]
   // TERM_DEBUG:     [[TERM_LPAD]]
   // TERM_DEBUG:     call void @__clang_call_terminate
   // TERM_DEBUG:     unreachable
   foo();
 }
-// TERM_DEBUG-DAG: [[DBG_LOC_START]] = !DILocation(line: [[@LINE-12]],
+// TERM_DEBUG-DAG: [[DBG_LOC_START]] = !DILocation(line: [[@LINE-14]],
 // TERM_DEBUG-DAG: [[DBG_LOC_END]] = !DILocation(line: [[@LINE-3]],
 #endif

clang/test/OpenMP/openmp_win_codegen.cpp

#pragma omp critical
   return 0;
 }
 
 // CHECK: define internal void [[OUTLINED]](
 // CHECK: invoke void @{{.+}}foo
 // CHECK: [[CATCHSWITCH:%.+]] = catchswitch within none
 // CHECK: [[CATCHPAD:%.+]] = catchpad within [[CATCHSWITCH]]
 // CHECK: call void @__kmpc_critical(%struct.ident_t* {{.*}}@0, i32 [[GID:%.+]],
+// CHECK-NEXT: fence acquire
 // CHECK: invoke void @{{.+}}bar
 // CHECK: call void @__kmpc_end_critical(%struct.ident_t* {{.*}}@0, i32 [[GID]],
 // CHECK: catchret from [[CATCHPAD]] to
 // CHECK:      cleanuppad within [[CATCHPAD]] []
+// CHECK-NEXT: fence release
 // CHECK-NEXT: call void @__kmpc_end_critical(%struct.ident_t* {{.*}}@0, i32 [[GID]],
 // CHECK-NEXT: cleanupret from {{.*}} unwind label %[[CATCHTERM:[^ ]+]]
 // CHECK:      cleanuppad within none []
 // CHECK-NEXT: call void @"?terminate@@YAXXZ"() #{{[0-9]+}} [ "funclet"(token %{{.*}}) ]
 // CHECK-NEXT: unreachable
 // CHECK:      [[CATCHTERM]]
 // CHECK-NEXT: cleanuppad within [[CATCHPAD]] []
 // CHECK-NEXT: call void @"?terminate@@YAXXZ"() #{{[0-9]+}} [ "funclet"(token %{{.*}}) ]
 // CHECK-NEXT: unreachable

clang/test/OpenMP/simd_codegen.cpp

 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -fexceptions -fcxx-exceptions -o - -fopenmp-version=45 | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s -fopenmp-version=45
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -debug-info-kind=limited -std=c++11 -include-pch %t -verify %s -emit-llvm -o - -fopenmp-version=45 | FileCheck %s
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fexceptions -fcxx-exceptions -debug-info-kind=line-tables-only -x c++ -emit-llvm %s -o - -fopenmp-version=45 | FileCheck %s --check-prefix=TERM_DEBUG
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -fexceptions -fcxx-exceptions -o - -fopenmp-version=50 -DOMP5 | FileCheck %s --check-prefix=CHECK --check-prefix=OMP50 --check-prefix=OMP50RT
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s -fopenmp-version=50 -DOMP5
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -debug-info-kind=limited -std=c++11 -include-pch %t -verify %s -emit-llvm -o - -fopenmp-version=50 -DOMP5 | FileCheck %s --check-prefix=CHECK --check-prefix=OMP50 --check-prefix=OMP50RT
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -fexceptions -fcxx-exceptions -o - -fopenmp-version=45 | FileCheck %s
// CHECK-NOT: mul i32 %{{.+}}, 10
   #pragma omp simd lastprivate(CONDITIONAL A)
 // CHECK: store i64 0, i64* [[OMP_IV7:%[^,]+]]
 // CHECK: br label %[[SIMD_LOOP7_COND:[^,]+]]
 // CHECK: [[SIMD_LOOP7_COND]]:
 // CHECK-NEXT: [[IV7:%.+]] = load i64, i64* [[OMP_IV7]]{{.*}}!llvm.access.group
 // CHECK-NEXT: [[CMP7:%.+]] = icmp slt i64 [[IV7]], 7
 // CHECK-NEXT: br i1 [[CMP7]], label %[[SIMPLE_LOOP7_BODY:.+]], label %[[SIMPLE_LOOP7_END:[^,]+]]
   for (long long i = -10; i < 10; i += 3) {
 // CHECK: [[SIMPLE_LOOP7_BODY]]:
 // Start of body: calculate i from IV:
 // CHECK: [[IV7_0:%.+]] = load i64, i64* [[OMP_IV7]]{{.*}}!llvm.access.group
 // CHECK-NEXT: [[LC_IT_1:%.+]] = mul nsw i64 [[IV7_0]], 3
 // CHECK-NEXT: [[LC_IT_2:%.+]] = add nsw i64 -10, [[LC_IT_1]]
 // CHECK-NEXT: store i64 [[LC_IT_2]], i64* [[LC:%[^,]+]],{{.+}}!llvm.access.group
 // CHECK-NEXT: [[LC_VAL:%.+]] = load i64, i64* [[LC]]{{.+}}!llvm.access.group
 // CHECK-NEXT: [[A_VAL:%.+]] = load i32, i32* [[A_PRIV:%[^,]+]],{{.+}}!llvm.access.group
 // CHECK-NEXT: [[CAST:%.+]] = sext i32 [[A_VAL]] to i64
 // CHECK-NEXT: [[ADD:%.+]] = add nsw i64 [[CAST]], [[LC_VAL]]
 // CHECK-NEXT: [[CONV:%.+]] = trunc i64 [[ADD]] to i32
 // CHECK-NEXT: store i32 [[CONV]], i32* [[A_PRIV]],{{.+}}!llvm.access.group
 // OMP50-NEXT: [[IV:%.+]] = load i64, i64* [[OMP_IV7]],{{.+}}!llvm.access.group
 // OMP50RT:    call void @__kmpc_critical(%struct.ident_t* {{.+}}, i32 [[GTID:%.+]], [8 x i32]* [[A_REGION:@.+]]),{{.+}}!llvm.access.group
+// OMP50RT-NEXT: fence acquire
 // OMP50-NEXT: [[LAST_IV_VAL:%.+]] = load i64, i64* [[LAST_IV]],{{.+}}!llvm.access.group
 // OMP50-NEXT: [[CMP:%.+]] = icmp sle i64 [[LAST_IV_VAL]], [[IV]]
 // OMP50-NEXT: br i1 [[CMP]], label %[[LP_THEN:.+]], label %[[LP_DONE:[^,]+]]
 // OMP50:      [[LP_THEN]]:
 // OMP50-NEXT: store i64 [[IV]], i64* [[LAST_IV]],{{.+}}!llvm.access.group
 // OMP50-NEXT: [[A_VAL:%.+]] = load i32, i32* [[A_PRIV]],{{.+}}!llvm.access.group
 // OMP50-NEXT: store i32 [[A_VAL]], i32* [[LAST_A]],{{.+}}!llvm.access.group
 // OMP50-NEXT: br label %[[LP_DONE]]
 // OMP50:      [[LP_DONE]]:
+// OMP50RT-NEXT: fence release
 // OMP50RT-NEXT: call void @__kmpc_end_critical(%struct.ident_t* {{.+}}, i32 [[GTID]], [8 x i32]* [[A_REGION]]),{{.+}}!llvm.access.group
     A += i;
 // CHECK: [[IV7_2:%.+]] = load i64, i64* [[OMP_IV7]]{{.*}}!llvm.access.group
 // CHECK-NEXT: [[ADD7_2:%.+]] = add nsw i64 [[IV7_2]], 1
 // CHECK-NEXT: store i64 [[ADD7_2]], i64* [[OMP_IV7]]{{.*}}!llvm.access.group
   }
 // CHECK: [[SIMPLE_LOOP7_END]]:
 // CHECK-NEXT: store i64 11, i64*

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

private:
   /// Common interface to finalize the region
   ///
   /// \param OMPD Directive to generate exiting code for
   /// \param FinIP Insertion point for emitting Finalization code and exit call
   /// \param ExitCall Call to the ending OMP Runtime Function
   /// \param HasFinalize indicate if the directive will require finalization
   ///         and has a finalization callback in the stack that
   ///        should be called.
+  /// \param WithMemBarrier true if the memory fence instructions are required
+  /// to prevent optimizations.
   ///
   /// \return The insertion position in exit block
   InsertPointTy emitCommonDirectiveExit(omp::Directive OMPD,
                                         InsertPointTy FinIP,
                                         Instruction *ExitCall,
-                                        bool HasFinalize = true);
+                                        bool HasFinalize = true,
+                                        bool WithMemBarrier = false);
 
   /// Common Interface to generate OMP inlined regions
   ///
   /// \param OMPD Directive to generate inlined region for
   /// \param EntryCall Call to the entry OMP Runtime Function
   /// \param ExitCall Call to the ending OMP Runtime Function
   /// \param BodyGenCB Body code generation callback.
   /// \param FiniCB Finalization Callback. Will be called when finalizing region
   /// \param Conditional indicate if the entry call result will be used
   ///        to evaluate a conditional of whether a thread will execute
   ///        body code or not.
   /// \param HasFinalize indicate if the directive will require finalization
   ///         and has a finalization callback in the stack that
   /// should        be called.
+  /// \param WithMemBarrier true if the memory fence instructions are required
+  /// to prevent optimizations.
   ///
   /// \return The insertion point after the region
 
   InsertPointTy
   EmitOMPInlinedRegion(omp::Directive OMPD, Instruction *EntryCall,
                        Instruction *ExitCall, BodyGenCallbackTy BodyGenCB,
                        FinalizeCallbackTy FiniCB, bool Conditional = false,
-                       bool HasFinalize = true);
+                       bool HasFinalize = true, bool WithMemBarrier = false);
 
   /// Get the platform-specific name separator.
   /// \param Parts different parts of the final name that needs separation
   /// \param FirstSeparator First separator used between the initial two
   ///        parts of the name.
   /// \param Separator separator used between all of the rest consecutinve
   ///        parts of the name
   static std::string getNameWithSeparators(ArrayRef<StringRef> Parts,

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::CreateCritical(
   }
   Instruction *EntryCall = Builder.CreateCall(RTFn, EnterArgs);
 
   Function *ExitRTLFn =
       getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_end_critical);
   Instruction *ExitCall = Builder.CreateCall(ExitRTLFn, Args);
 
   return EmitOMPInlinedRegion(OMPD, EntryCall, ExitCall, BodyGenCB, FiniCB,
-                              /*Conditional*/ false, /*hasFinalize*/ true);
+                              /*Conditional*/ false, /*hasFinalize*/ true,
+                              /*WithMemBarrier=*/true);
 }
 
 OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::EmitOMPInlinedRegion(
     Directive OMPD, Instruction *EntryCall, Instruction *ExitCall,
     BodyGenCallbackTy BodyGenCB, FinalizeCallbackTy FiniCB, bool Conditional,
-    bool HasFinalize) {
+    bool HasFinalize, bool WithMemBarrier) {
 
   if (HasFinalize)
     FinalizationStack.push_back({FiniCB, OMPD, /*IsCancellable*/ false});
 
   // Create inlined region's entry and body blocks, in preparation
   // for conditional creation
   BasicBlock *EntryBB = Builder.GetInsertBlock();
   Instruction *SplitPos = EntryBB->getTerminator();
   if (!isa_and_nonnull<BranchInst>(SplitPos))
     SplitPos = new UnreachableInst(Builder.getContext(), EntryBB);
   BasicBlock *ExitBB = EntryBB->splitBasicBlock(SplitPos, "omp_region.end");
   BasicBlock *FiniBB =
       EntryBB->splitBasicBlock(EntryBB->getTerminator(), "omp_region.finalize");
 
   Builder.SetInsertPoint(EntryBB->getTerminator());
   emitCommonDirectiveEntry(OMPD, EntryCall, ExitBB, Conditional);
+  if (WithMemBarrier)
+    Builder.CreateFence(AtomicOrdering::Acquire);
 
   // generate body
   BodyGenCB(/* AllocaIP */ InsertPointTy(),
             /* CodeGenIP */ Builder.saveIP(), *FiniBB);
 
   // If we didn't emit a branch to FiniBB during body generation, it means
   // FiniBB is unreachable (e.g. while(1);). stop generating all the
   // unreachable blocks, and remove anything we are not going to use.
   auto SkipEmittingRegion = FiniBB->hasNPredecessors(0);
   if (SkipEmittingRegion) {
     FiniBB->eraseFromParent();
     ExitCall->eraseFromParent();
     // Discard finalization if we have it.
     if (HasFinalize) {
       assert(!FinalizationStack.empty() &&
              "Unexpected finalization stack state!");
       FinalizationStack.pop_back();
     }
   } else {
     // emit exit call and do any needed finalization.
     auto FinIP = InsertPointTy(FiniBB, FiniBB->getFirstInsertionPt());
     assert(FiniBB->getTerminator()->getNumSuccessors() == 1 &&
            FiniBB->getTerminator()->getSuccessor(0) == ExitBB &&
            "Unexpected control flow graph state!!");
-    emitCommonDirectiveExit(OMPD, FinIP, ExitCall, HasFinalize);
+    emitCommonDirectiveExit(OMPD, FinIP, ExitCall, HasFinalize, WithMemBarrier);
     assert(FiniBB->getUniquePredecessor()->getUniqueSuccessor() == FiniBB &&
            "Unexpected Control Flow State!");
     MergeBlockIntoPredecessor(FiniBB);
   }
 
   // If we are skipping the region of a non conditional, remove the exit
   // block, and clear the builder's insertion point.
   assert(SplitPos->getParent() == ExitBB &&
OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::emitCommonDirectiveEntry(
   Builder.SetInsertPoint(ThenBB->getTerminator());
 
   // return an insertion point to ExitBB.
   return IRBuilder<>::InsertPoint(ExitBB, ExitBB->getFirstInsertionPt());
 }
 
 OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::emitCommonDirectiveExit(
     omp::Directive OMPD, InsertPointTy FinIP, Instruction *ExitCall,
-    bool HasFinalize) {
+    bool HasFinalize, bool WithMemBarrier) {
 
   Builder.restoreIP(FinIP);
 
   // If there is finalization to do, emit it before the exit call
   if (HasFinalize) {
     assert(!FinalizationStack.empty() &&
            "Unexpected finalization stack state!");
 
     FinalizationInfo Fi = FinalizationStack.pop_back_val();
     assert(Fi.DK == OMPD && "Unexpected Directive for Finalization call!");
 
     Fi.FiniCB(FinIP);
 
     BasicBlock *FiniBB = FinIP.getBlock();
     Instruction *FiniBBTI = FiniBB->getTerminator();
 
     // set Builder IP for call creation
     Builder.SetInsertPoint(FiniBBTI);
   }
 
+  if (WithMemBarrier)
+    Builder.CreateFence(AtomicOrdering::Release);
   // place the Exitcall as last instruction before Finalization block terminator
   ExitCall->removeFromParent();
   Builder.Insert(ExitCall);
 
   return IRBuilder<>::InsertPoint(ExitCall->getParent(),
                                   ExitCall->getIterator());
 }