[OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

[fghanim]

Summary

Add support for Master and Critical directive in the OMPIRBuilder. Both make use of a new common interface for emitting inlined OMP regions called emitInlinedRegion which was added in this patch as well.

Also this patch modifies clang to use the new directives when -fopenmp-enable-irbuilder commandline option is passed.

[jdoerfert]

Some initial comments. Can we split it in two patches (master/critical)?

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

282

If there is no good reason agains it, these should go into the OMPKinds.def/OMPConstants.{h/cpp}. We need support for array types but that is not a problem.

291

A lot of the functions below should not be public. We should expose as little as possible, mainly the CreateDirective methods.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

735

You can just write OMPD_master below instead of OMPD.

745

Make sure the patch is clang formatted. See also: https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting

939

Is this switch + if-cascade + callEntry/Exit really helpful?
I mean we can replace

`Instruction *EntryCall = CreateEntryCall(OMPD, Args);`

with

`Instruction *EntryCall = Builder.CreateCall(getOrCreateRuntimeFunction(OMPRTL___kmpc_omp_master), Args);`

right?

[fghanim]

I don't think there is enough in this patch to split it into two or three patches. The main part is EmitOMPInlinedRegion which does all the heavy lifting. At this point, both create Master & Critical are almost wrappers collecting arguments for EmitOMPInlinedRegion. So I really do not want to split them into multiple patches unless I have to.

As for the rest , I just wanted to provide my thinking when I did them. I'll update them if you think it's better to. let me know.

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

282

I tried to do that, but started to get compilation errors when I put them in OMPConstants.{h/cpp} . So I thought that given that these are used exactly once according to current implementation in clang (i.e. for 'critical name'). I'll do it as part of OMPBuilder for the time being.

291

I think the OMPbuilder should provide people writing llvm transformation passes (e.g. autoparallelizing) with a way to be able to emit correct OMP calls based on their needs.

If you have minor suggestions to make it better, I'd love to update this based on that.
Alternatively, I'll make them private for now, until we decide on something for that.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

735

Yes. I just feel this is a bit cleaner is all.

745

Will do

939

While I completely agree that the switch+if-cascade is not an elegant solution, It's helpful in enabling a generic way to the directive entry and exit calls. which is in turn helpful for EmitCommonDirectiveEntry and EmitCommonDirectiveExit.

FWIW, CreateEntryCall was used inside of EmitCommonDirectiveEntry, similar to how CreateExitCall is currently inside EmitCommonDirectiveExit. RTLFuncName enabled that. I only took it out because of Critical's Entry_Hint special case. I plan to put it back in, If critical is the only directive like that.

I am not too attached to it, but I think it's help-ing :)

[jdoerfert]

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

282

These are not good arguments. We have *all other types* defined in OMPKinds.def + OMPConstants.{h/cpp}, doing something different here just complicates the code.

291

It is futile to design based on some use cases we cannot describe properly. Once the need arises for public interfaces, the callType (inconsistent spelling!), the Entry/Exit call stuff, we can add it but not before.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

735

I argue it is harder to read the code this way (especially at the call site below). Though, I won't make the change a requirement.

939

What is EmitCommonDirectiveEntry and EmitCommonDirectiveExit ?

I don't see how it is helping to have emitEntry(createEntry(DIRECTIVE)) instead of emit(DIRECTIVE_ENTRY), especially since the former needs all this logic to map DIRECTIVE to DIRECTIVE_ENTRY. The latter shows you at the call site, e.g., in CreateCritical, exactly what runtime call is being created and at these call sites we already know exactly what runtime call we need.

You can still have some helper that puts entry/exit around some code, simply pass both the entry and exit runtime call ID instead of just the directive ID from which you would "compute" the entry/exit runtime call ID.

---

Long story short, if making things explicit minimizes code complexity along the way I'm all for it.

[jdoerfert]

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

934

While I hope this code goes away, I should mention that we don't write assert(false && "...") but llvm_unreachable("...").

[fghanim]

• Adding array types to OMPKinds.def. Inlining runtime function calls generation. reformatting with clang format

[fghanim]

So I modified the patch based on your comments. I removed all the code that I intended for llvm pass writers for now.
I will look at it again later, figure something to do with the whole switch-if-cascade thing, and will introduce it in a separate patch. I think that would be better than to include it here.

[jdoerfert]

I added quite a bunch of inline comments again :( and some high level stuff below.

---

Commits need a message that explains the change.
Also the "Summary" should not be in the commit title.

---

This adds support for clang to build the directives via the OMPIRBuilder so we need to add tests for that. Probably "just" running existing codegen tests for the directives with the OMPIRBuidlder enabled.

clang/lib/CodeGen/CGStmtOpenMP.cpp

3161

Can you add an assertion that the branch has only a single successor.

3165

IPBBTI->eraseFromParent()

3176

Why the isSet check? If we need it, then it's missing in other places as well (I think).

3228

Can we do

llvm::Value *HintInst = nullptr;
if (Hint)
  ...

these three lines look ugly :(

3244

I somehow have the feeling we could have a "FiniCB" helper function as they seem to be always the same. A TODO mentioning that is also OK for now.

3267

Same here, that looks like the other "BodyGenCB". We should really make them helpers.

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

292

FiniCB param docu is missing.

328

Conditional

345

HasFinalize

364

Format: no newline, above the comments got broken

382

All methods need documentation of some kind. llvm:: is not needed here. It seems one method is declared twice.

llvm/lib/Frontend/OpenMP/OMPConstants.cpp

66

These comments that reference KmpCriticalNameTy do not help here. If you want, feel free to add comments that explain how the result looks in a generic way but only showing KmpCriticalNameTy seems counterproductive to me.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

768

Spelling: Fn

789

Spelling Conditional, HasFinalize

794

Outdated comment.

811

Nit: InsertPointTy FiniIP(FiniBB, FiniBB->getFirstInsertionPt());

815

Shouldn't we use the argument here?

822

Style: No braces

831

Conditional, here and elsewhere no llvm:: (and omp::)

833

Naming: ContPt or ContIP or ...

860

I usually have neither or both conditional branches in braces. Here we can opt for an early exist (preferred anyway) and get:

if (!Conditional)
  return Builder.SaveIP();
...

Some variables can then be moved past this early exit

884

extra parenthesis, and, actually, this condition can not happen (I think). There is always something at the end of the block or your get an error earlier.

894

ExitCall->moveBefore(IP), potentially with some IP = ... above.
We should specify what the builder insert position is after this call, maybe "unspecified"?

902

This can be a static helper and the name should be more descriptive, getName is very generic.

918

This seems to be too much trouble to get a StringRef from a Twine. Maybe start with a StringRef and avoid all of it?

930

Do we really want common linkage for "internal variables"? Internal or private would have been my first guess. Maybe the "internal" part is different than I expect it.

---

FWIW, the usual pattern is

if (!Elem.second)
  Elem.second = ...
assert(...)
return Elem.second;

[fghanim]

Commits need a message that explains the change.
Also the "Summary" should not be in the commit title.

Is this something for future commits or do you want me to change these?
If the latter, how do I do that?

This adds support for clang to build the directives via the OMPIRBuilder so we need to add tests for that. Probably "just" running existing codegen tests for the directives with the OMPIRBuidlder enabled.

You mean unit tests? or something else? If not unit tests, Where do I do that?

As for rest of comments, anything without response, is something I will do.

clang/lib/CodeGen/CGStmtOpenMP.cpp

3176

While I was writing the response I realized that I should've made it
assert((!AllocaIP.isSet() || AllocaInsertPt == AllocaIP.getpoint()) && "Inlined directives allocas should be emitted to entry block of the inner most containing outlined function";
or something less verbose.

Inlined directives should emit their allocas to the entry block of the containing outined function, right? For that reason, and since we don't store an alloca insertion point in the OMPBuilder, I pass an empty insertion point, and back up the current point, without changing it.
I was planning, in the future, to keep a copy of the most recent alloca insertion point which we set when we outline, and pass that for BodyCBs within inlined directives.

3228

np.

btw, the todo note is about the CGM.Int32Ty. Currently in clang it's CGM.IntTy. I'll expand on the todo

3244

I'll do a Todo for now. I am using a very similar FiniCB to the one you used for parallel. So it doesn't make sense to change these here and not that one with them.

3267

Same as prev.

llvm/lib/Frontend/OpenMP/OMPConstants.cpp

66

I was testing something, and I forgot to remove them when I was done.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

811

Nit?

815

"The argument" refers to?

[fghanim]

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

902

This is taken from clang (I think CG_OMP) as is. I am keeping everything as is for now in case somebody decided to help and came across it being used somewhere else, they should recognize it immediately.

So for now, I suggest a todo. cos if I find that it's not used except with critical, I'll merge all 3 functions into 1 and be done with it.

918

Same as prev.

930

same as prev.

[jdoerfert]

In D72304#1815793, @fghanim wrote:

Commits need a message that explains the change.
Also the "Summary" should not be in the commit title.

Is this something for future commits or do you want me to change these?
If the latter, how do I do that?

Already for this one, at least the git commit.
You can edit this patch in phab (top right, edit revision) and you need to make sure to adjust the git commit before pushing.

This adds support for clang to build the directives via the OMPIRBuilder so we need to add tests for that. Probably "just" running existing codegen tests for the directives with the OMPIRBuidlder enabled.

You mean unit tests? or something else? If not unit tests, Where do I do that?

We can use lit test for that. The two files that you need to modify for sure are clang/test/OpenMP/critical_codegen.cpp and clang/test/OpenMP/master_codegen.cpp. You add run lines so that the code in clang is actually executed and modify the check lines appropriately.
I did split my commits into the llvm and clang part mostly, if you look at a clang patch which uses the OpenMPIRBuilder, you should see what I mean.

As for rest of comments, anything without response, is something I will do.

I did respond to the ones with.

clang/lib/CodeGen/CGStmtOpenMP.cpp

3176

I try not to make assumptions like "Inlined directives should emit their allocas to the entry block of the containing outined function, right" if we don't need to. If we do, the assert is probably a better way to go. I would actually say we want a helper that sets/resets all these things for *all* body functions. If the OMPBuilder doesn't want to set a new allocaip, it should provide an "invalid" location to indicate that. Now I finally see what you do here and why. Go with the assert for now, and make it !isSet() if there is no reason to complicate it further.

3244

Agreed. We need a helper that is used at both/all places.

3267

Again a TODO and we are good for now.

llvm/lib/Frontend/OpenMP/OMPConstants.cpp

66

I take it you'll remove them. Just saying because you commented ;)

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

811

Nit is often used to indicate a request that can be ignored silently (https://www.urbandictionary.com/define.php?term=nit)

815

Sorry. I mean passing hasFinalize instead of true

902

You can keep it a separate helper but I still don't see how it helps to keep this a member (instead of a static).

I also want the name change. My rational is simple: If we pick a name that is meaningful, people know what it does regardless of whether they know the version in clang.

918

Here, the argument that people recognize it doesn't work at all, it's a type. So please pass a StringRef if there is no other reason not to.

930

The code pattern is unrelated from "same as prev", I hope.

For the linkage, add a TODO to investigate if private linkage would work as well.

[fghanim]

I am working on a patch. In the mean time ;)

clang/lib/CodeGen/CGStmtOpenMP.cpp

3176

I think most (if not all) llvm passes that are concerned with allocas, expect them to be in the entry block, this is why I made this assumption. The only time that I can think of, where this will not be true is in case of a delayed outlining. In that case, the insertion block is going to be the "future" entry block of the "future" outlined function, and needs to be passed. Then I think it is also necessary to keep track of the insertion block somewhere inside the OMPBuilder.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

894

before this point, the builder is always set to insert before the terminator of the finalization block. when emitInlinedFunction is done, it sets it to ExitBB.end()

918

You are right. *That* argument doesn't work. However the argument that I found it this way in clang kinda does. Also the argument that it makes things much easier to move. ;)
I assume whoever did it this way had his/her good reason. I understand you may not think it is, but given that it's used a lot in clang for getting and creating internal OMP variables, you can't really judge without going over every case to make sure that what you want can be done in every case, as such I am against the change *NOW*. However, for the time being, I included a todo basically saying that.

930

The assert is only needed if Elem.second already exist. It's quite useless if it's not and we are creating it, right?
I'll change it to an if..else, and return it once.

critical name -which is what this is being used for now- has an external linkage, isn't it?
Also, as I said this is being used by many others all over CG_OMP to create OMP internal variables.
Maybe later we'll create different versions for different internal variables, but for now I'll add a todo anyway.

[fghanim]

Addressing reviewer comments

• Added regression tests
• styling
• adding asserts and todo where needed.

Also, Now EmitInlinedRegion will merge blocks where legal.

[jdoerfert]

LGTM with some minor things you should address before the merge.

llvm/include/llvm/Frontend/OpenMP/OMPConstants.h

100

Leftover comment.

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

305

Nit: typos in parameter name and comment, comments seems outdated.

338

Nit: Comment is not in sync with the Entry one.
Nit: spelling in the parameter name and format.

359

Nit: spelling param name and format.

llvm/lib/Frontend/OpenMP/OMPConstants.cpp

45

Leftover

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

789

This needs to be guarded by HasFinalize if the corresponding pop is.

843

I have the feeling the merging makes it harder without providing a benefit but I'm OK with it for now

853

omp is not a good name here.

885

HasFinalize

[fghanim]

Addressing reviewer's comments

• fixed styling and naming according to llvm conventions

[fghanim]

Thanks for reviewing this.

After I address the last two comments below, How do I merge with llvm using phab? I'd appreciate an llvm specific link if possible.

llvm/lib/Frontend/OpenMP/OMPConstants.cpp

45

what is this referring to?

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

843

more difficult in what way?

[jdoerfert]

Phab is not involved in the merge process, it just is a normal git push on top of the llmv-project.
Do you have commit rights already? If not I can commit this for you in a few days.

llvm/lib/Frontend/OpenMP/OMPConstants.cpp

45

You deleted an empty new line ;)

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

843

We need things like auto InsertBB = merged ? ExitPredBB : ExitBB; now for example.

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

695

Nit: The else is not needed.

754

Nit: The else is not needed.

771

Nit: The else is not needed.

[jdoerfert]

The commit title has a stray { instead of [

[fghanim]

Addressing reviewer's comments regarding styling.

[fghanim]

No, I don't have commit privileges. I'd appreciate if you'd commit this for me. Thanks :)

[jdoerfert]

I tried to push this today but the build failed:

/data/src/llvm-project/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:252:36: error: use of undeclared identifier 'BumpPtrAllocator'
  StringMap<AssertingVH<Constant>, BumpPtrAllocator> InternalVars;
                                   ^
/data/src/llvm-project/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:832:10: error: no viable conversion from returned value of type 'llvm::StringRef' to function return type 'std::string' (aka 'basic_string<char>')
  return OS.str();
         ^~~~~~~~

Can you look into that please?

[fghanim]

Thanks for trying.

It compiles fine with me. How can I reproduce the problems you're getting? is there a way to pack what you have and send it to me so I can have a look?

Also, a second question, how trustworthy is harbormaster when it says something is buildable?

[jdoerfert]

In D72304#1846115, @fghanim wrote:

Thanks for trying.

It compiles fine with me. How can I reproduce the problems you're getting? is there a way to pack what you have and send it to me so I can have a look?

Did you rebase onto the newest llvm master?

Also, a second question, how trustworthy is harbormaster when it says something is buildable?

Honestly, I do not know.

[fghanim]

Everything is fine!
I just cloned llvm from git hub, added this revision with arc patch. then:
cmake -G"Unix Makefiles" -DLLVM_ENABLE_PROJECTS="clang;openmp" -DCMAKE_BUILD_TYPE=Release -DLLVM_OPTIMIZED_TABLEGEN=1 -DLLVM_USE_LINKER=gold ../llvm-project/llvm
make , make check-llvm-unittest, make check-clang-openmp

all of it works fine and I got no errors whatsoever. Let me know what you think.

[fghanim]

ping

[jdoerfert]

Reverted in 9dcfc7cd64abb301124cafaa95661b76a1fc5032 because of breakages and warnings:
http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/22899

[fghanim]

Addressing errors/warnings from commit build

[fghanim]

ping

[kadircet]

this breaks under asan http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/38597/steps/check-clang%20asan/logs/stdio

[fghanim]

fixing unused variable.

[fghanim]

Remove unnecessary preservation of builder insertion point in emitCommonDirectiveExit

[jdoerfert]

I did run clang-format on the OMPIRBuilder files and replaced tabs with spaces. I think you need to teach your editor to use spaces in the future.

[MaskRay]

test/CodeGen/opt-record-1.c triggers an assertion failure.

[jdoerfert]

In D72304#1877676, @MaskRay wrote:

test/CodeGen/opt-record-1.c triggers an assertion failure.

I hope the macro reordering in rGa0236de7a927 did the trick.

[fhahn]

It looks like this patch (or the other related changes committed recently) may cause ASan failures running Transforms/OpenMP/gtid.ll and Transforms/OpenMP/parallel_deletion.ll http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6960/consoleFull

It would be great if you could take a look and possibly revert the change if it takes some time to investigate.

[jdoerfert]

In D72304#1877855, @fhahn wrote:

It looks like this patch (or the other related changes committed recently) may cause ASan failures running Transforms/OpenMP/gtid.ll and Transforms/OpenMP/parallel_deletion.ll http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6960/consoleFull

It would be great if you could take a look and possibly revert the change if it takes some time to investigate.

Should already be fixed with rGa0236de7a927.

I wait for http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6961/ to come back.

clang/lib/CodeGen/CGStmtOpenMP.cpp

static void emitMaster(CodeGenFunction &CGF, const OMPExecutableDirective &S) {
   auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &Action) {
     Action.Enter(CGF);
     CGF.EmitStmt(S.getInnermostCapturedStmt()->getCapturedStmt());
   };
   CGF.CGM.getOpenMPRuntime().emitMasterRegion(CGF, CodeGen, S.getBeginLoc());
 }
 
 void CodeGenFunction::EmitOMPMasterDirective(const OMPMasterDirective &S) {
+  if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder()) {
+    using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+
+    const CapturedStmt *CS = S.getInnermostCapturedStmt();
+    const Stmt *MasterRegionBodyStmt = CS->getCapturedStmt();
+
+    // TODO: Replace with a generic helper function for finalization
+    auto FiniCB = [this](InsertPointTy IP) {
+      CGBuilderTy::InsertPointGuard IPG(Builder);
+      assert(IP.getBlock()->end() != IP.getPoint() &&
+             "OpenMP IR Builder should cause terminated block!");
+
+      llvm::BasicBlock *IPBB = IP.getBlock();
+      llvm::BasicBlock *DestBB = IPBB->getUniqueSuccessor();
+      assert(DestBB && "Finalization block should have one successor!");
+
+      // erase and replace with cleanup branch.
+      IPBB->getTerminator()->eraseFromParent();
+      Builder.SetInsertPoint(IPBB);
+      CodeGenFunction::JumpDest Dest = getJumpDestInCurrentScope(DestBB);
+      EmitBranchThroughCleanup(Dest);
+    };
+
+    // TODO: Replace with a generic helper function for emitting body
+    auto BodyGenCB = [MasterRegionBodyStmt, this](InsertPointTy AllocaIP,
+                                                  InsertPointTy CodeGenIP,
+                                                  llvm::BasicBlock &FiniBB) {
+      // Alloca insertion block should be in the entry block of the containing
+      // function So it expects an empty AllocaIP in which case will reuse the
+      // old alloca insertion point, or a new AllocaIP in the same block as the
+      // old one
+      assert((!AllocaIP.isSet() ||
+              AllocaInsertPt->getParent() == AllocaIP.getBlock()) &&
+             "Insertion point should be in the entry block of containing "
+             "function!");
+      auto OldAllocaIP = AllocaInsertPt;
+      if (AllocaIP.isSet())
+        AllocaInsertPt = &*AllocaIP.getPoint();
+      auto OldReturnBlock = ReturnBlock;
+      ReturnBlock = getJumpDestInCurrentScope(&FiniBB);
+
+      llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
+      if (llvm::Instruction *CodeGenIPBBTI = CodeGenIPBB->getTerminator())
+        CodeGenIPBBTI->eraseFromParent();
+
+      Builder.SetInsertPoint(CodeGenIPBB);
+
+      EmitStmt(MasterRegionBodyStmt);
+
+      if (Builder.saveIP().isSet())
+        Builder.CreateBr(&FiniBB);
+
+      AllocaInsertPt = OldAllocaIP;
+      ReturnBlock = OldReturnBlock;
+    };
+    CGCapturedStmtInfo CGSI(*CS, CR_OpenMP);
+    CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
+    Builder.restoreIP(OMPBuilder->CreateMaster(Builder, BodyGenCB, FiniCB));
+
+    return;
+  }
   OMPLexicalScope Scope(*this, S, OMPD_unknown);
   emitMaster(*this, S);
 }
 
 void CodeGenFunction::EmitOMPCriticalDirective(const OMPCriticalDirective &S) {
+  if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder()) {
+    using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+
+    const CapturedStmt *CS = S.getInnermostCapturedStmt();
+    const Stmt *CriticalRegionBodyStmt = CS->getCapturedStmt();
+    const Expr *Hint = nullptr;
+    if (const auto *HintClause = S.getSingleClause<OMPHintClause>())
+      Hint = HintClause->getHint();
+
+    // TODO: This is slightly different from what's currently being done in
+    // clang. Fix the Int32Ty to IntPtrTy (pointer width size) when everything
+    // about typing is final.
+    llvm::Value *HintInst = nullptr;
+    if (Hint)
+      HintInst =
+          Builder.CreateIntCast(EmitScalarExpr(Hint), CGM.Int32Ty, false);
+
+    // TODO: Replace with a generic helper function for finalization
+    auto FiniCB = [this](InsertPointTy IP) {
+      CGBuilderTy::InsertPointGuard IPG(Builder);
+      assert(IP.getBlock()->end() != IP.getPoint() &&
+             "OpenMP IR Builder should cause terminated block!");
+      llvm::BasicBlock *IPBB = IP.getBlock();
+      llvm::BasicBlock *DestBB = IPBB->getUniqueSuccessor();
+      assert(DestBB && "Finalization block should have one successor!");
+
+      // erase and replace with cleanup branch.
+      IPBB->getTerminator()->eraseFromParent();
+      Builder.SetInsertPoint(IPBB);
+      CodeGenFunction::JumpDest Dest = getJumpDestInCurrentScope(DestBB);
+      EmitBranchThroughCleanup(Dest);
+    };
+
+    // TODO: Replace with a generic helper function for emitting body
+    auto BodyGenCB = [CriticalRegionBodyStmt, this](InsertPointTy AllocaIP,
+                                                    InsertPointTy CodeGenIP,
+                                                    llvm::BasicBlock &FiniBB) {
+      // Alloca insertion block should be in the entry block of the containing
+      // function So it expects an empty AllocaIP in which case will reuse the
+      // old alloca insertion point, or a new AllocaIP in the same block as the
+      // old one
+      assert((!AllocaIP.isSet() ||
+              AllocaInsertPt->getParent() == AllocaIP.getBlock()) &&
+             "Insertion point should be in the entry block of containing "
+             "function!");
+      auto OldAllocaIP = AllocaInsertPt;
+      if (AllocaIP.isSet())
+        AllocaInsertPt = &*AllocaIP.getPoint();
+      auto OldReturnBlock = ReturnBlock;
+      ReturnBlock = getJumpDestInCurrentScope(&FiniBB);
+
+      llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
+      if (llvm::Instruction *CodeGenIPBBTI = CodeGenIPBB->getTerminator())
+        CodeGenIPBBTI->eraseFromParent();
+
+      Builder.SetInsertPoint(CodeGenIPBB);
+
+      EmitStmt(CriticalRegionBodyStmt);
+
+      if (Builder.saveIP().isSet())
+        Builder.CreateBr(&FiniBB);
+
+      AllocaInsertPt = OldAllocaIP;
+      ReturnBlock = OldReturnBlock;
+    };
+
+    CGCapturedStmtInfo CGSI(*CS, CR_OpenMP);
+    CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
+    Builder.restoreIP(OMPBuilder->CreateCritical(
+        Builder, BodyGenCB, FiniCB, S.getDirectiveName().getAsString(),
+        HintInst));
+
+    return;
+  }
+
   auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &Action) {
     Action.Enter(CGF);
     CGF.EmitStmt(S.getInnermostCapturedStmt()->getCapturedStmt());
   };
   const Expr *Hint = nullptr;
   if (const auto *HintClause = S.getSingleClause<OMPHintClause>())
     Hint = HintClause->getHint();
   OMPLexicalScope Scope(*this, S, OMPD_unknown);

clang/test/OpenMP/critical_codegen.cpp

-// RUN: %clang_cc1 -verify -fopenmp -x c++ -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s --check-prefixes=ALL,NORMAL
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=ALL,NORMAL
 // 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 - | FileCheck %s --check-prefix=TERM_DEBUG
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-enable-irbuilder -x c++ -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s --check-prefixes=ALL,IRBUILDER
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=ALL,IRBUILDER
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp-simd -fexceptions -fcxx-exceptions -debug-info-kind=line-tables-only -x c++ -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER
 
-// CHECK:       [[IDENT_T_TY:%.+]] = type { i32, i32, i32, i32, i8* }
-// CHECK:       [[UNNAMED_LOCK:@.+]] = common global [8 x i32] zeroinitializer
-// CHECK:       [[THE_NAME_LOCK:@.+]] = common global [8 x i32] zeroinitializer
-// CHECK:       [[THE_NAME_LOCK1:@.+]] = common global [8 x i32] zeroinitializer
+// ALL:       [[IDENT_T_TY:%.+]] = type { i32, i32, i32, i32, i8* }
+// ALL:       [[UNNAMED_LOCK:@.+]] = common global [8 x i32] zeroinitializer
+// ALL:       [[THE_NAME_LOCK:@.+]] = common global [8 x i32] zeroinitializer
+// ALL:       [[THE_NAME_LOCK1:@.+]] = common global [8 x i32] zeroinitializer
 
-// CHECK:       define {{.*}}void [[FOO:@.+]]()
+// ALL:       define {{.*}}void [[FOO:@.+]]()
 
 void foo() {}
 
-// CHECK-LABEL: @main
+// ALL-LABEL: @main
 // TERM_DEBUG-LABEL: @main
 int main() {
-// CHECK:       [[A_ADDR:%.+]] = alloca i8
+  // ALL:       [[A_ADDR:%.+]] = alloca i8
   char a;
 
-// CHECK:       [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
-// CHECK:       call {{.*}}void @__kmpc_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[UNNAMED_LOCK]])
-// CHECK-NEXT:  store i8 2, i8* [[A_ADDR]]
-// CHECK-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:        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]])
 #pragma omp critical
   a = 2;
-// CHECK:       call {{.*}}void @__kmpc_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[THE_NAME_LOCK]])
-// CHECK-NEXT:  invoke {{.*}}void [[FOO]]()
-// CHECK:       call {{.*}}void @__kmpc_end_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[THE_NAME_LOCK]])
+// 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]])
 #pragma omp critical(the_name)
   foo();
-// CHECK:       call {{.*}}void @__kmpc_critical_with_hint([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[THE_NAME_LOCK1]], i{{64|32}} 23)
-// CHECK-NEXT:  invoke {{.*}}void [[FOO]]()
-// CHECK:       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)
+// 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]])
 #pragma omp critical(the_name1) hint(23)
   foo();
-// CHECK:       call {{.*}}void @__kmpc_critical([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]], [8 x i32]* [[THE_NAME_LOCK]])
-// CHECK:       br label
-// CHECK-NOT:   call {{.*}}void @__kmpc_end_critical(
-// CHECK:       br label
-// CHECK-NOT:   call {{.*}}void @__kmpc_end_critical(
-// CHECK:       br label
+  // 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:       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)
       ;
-// CHECK:  call {{.*}}void [[FOO]]()
+  // ALL:  call {{.*}}void [[FOO]]()
   foo();
-// CHECK-NOT:   call void @__kmpc_critical
-// CHECK-NOT:   call void @__kmpc_end_critical
+  // ALL-NOT:   call void @__kmpc_critical
+  // ALL-NOT:   call void @__kmpc_end_critical
   return a;
 }
 
 struct S {
   int a;
 };
-// CHECK-LABEL: critical_ref
+// ALL-LABEL: critical_ref
 void critical_ref(S &s) {
-  // CHECK: [[S_ADDR:%.+]] = alloca %struct.S*,
-  // CHECK: [[S_REF:%.+]] = load %struct.S*, %struct.S** [[S_ADDR]],
-  // CHECK: [[S_A_REF:%.+]] = getelementptr inbounds %struct.S, %struct.S* [[S_REF]], i32 0, i32 0
+  // 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;
-  // CHECK: [[S_REF:%.+]] = load %struct.S*, %struct.S** [[S_ADDR]],
-  // CHECK: store %struct.S* [[S_REF]], %struct.S** [[S_ADDR:%.+]],
-  // CHECK: call void @__kmpc_critical(
+  // 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(
 #pragma omp critical
-  // CHECK: [[S_REF:%.+]] = load %struct.S*, %struct.S** [[S_ADDR]],
-  // CHECK: [[S_A_REF:%.+]] = getelementptr inbounds %struct.S, %struct.S* [[S_REF]], i32 0, i32 0
+  // 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;
-  // CHECK: call void @__kmpc_end_critical(
+  // ALL: call void @__kmpc_end_critical(
 }
 
-// CHECK-LABEL:      parallel_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:     invoke void {{.*}}foo{{.*}}()
   // TERM_DEBUG:     unwind label %[[TERM_LPAD:.+]],

clang/test/OpenMP/master_codegen.cpp

-// RUN: %clang_cc1 -verify -fopenmp -x c++ -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s --check-prefixes=ALL,NORMAL
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=ALL,NORMAL
 // 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 - | FileCheck %s --check-prefix=TERM_DEBUG
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-enable-irbuilder -x c++ -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s --check-prefixes=ALL,IRBUILDER
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=ALL,IRBUILDER
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp-simd -fexceptions -fcxx-exceptions -debug-info-kind=line-tables-only -x c++ -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER
 
-// CHECK:       [[IDENT_T_TY:%.+]] = type { i32, i32, i32, i32, i8* }
+// ALL:       [[IDENT_T_TY:%.+]] = type { i32, i32, i32, i32, i8* }
 
-// CHECK:       define {{.*}}void [[FOO:@.+]]()
+// ALL:       define {{.*}}void [[FOO:@.+]]()
 
 void foo() {}
 
-// CHECK-LABEL: @main
+// ALL-LABEL: @main
 // TERM_DEBUG-LABEL: @main
 int main() {
-  // CHECK:       [[A_ADDR:%.+]] = alloca i8
+  // ALL:           [[A_ADDR:%.+]] = alloca i8
   char a;
 
-// CHECK:       [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
-// CHECK:       [[RES:%.+]] = call {{.*}}i32 @__kmpc_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
-// CHECK-NEXT:  [[IS_MASTER:%.+]] = icmp ne i32 [[RES]], 0
-// CHECK-NEXT:  br i1 [[IS_MASTER]], label {{%?}}[[THEN:.+]], label {{%?}}[[EXIT:.+]]
-// CHECK:       [[THEN]]
-// CHECK-NEXT:  store i8 2, i8* [[A_ADDR]]
-// CHECK-NEXT:  call {{.*}}void @__kmpc_end_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
-// CHECK-NEXT:  br label {{%?}}[[EXIT]]
-// CHECK:       [[EXIT]]
+// ALL:             [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
+// ALL:             [[RES:%.+]] = call {{.*}}i32 @__kmpc_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
+// ALL-NEXT:        [[IS_MASTER:%.+]] = icmp ne i32 [[RES]], 0
+// ALL-NEXT:        br i1 [[IS_MASTER]], label {{%?}}[[THEN:.+]], label {{%?}}[[EXIT:.+]]
+// ALL:             [[THEN]]
+// ALL-NEXT:        store i8 2, i8* [[A_ADDR]]
+// ALL-NEXT:        call {{.*}}void @__kmpc_end_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
+// ALL-NEXT:        br label {{%?}}[[EXIT]]
+// ALL:             [[EXIT]]
 #pragma omp master
   a = 2;
-// CHECK:       [[RES:%.+]] = call {{.*}}i32 @__kmpc_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
-// CHECK-NEXT:  [[IS_MASTER:%.+]] = icmp ne i32 [[RES]], 0
-// CHECK-NEXT:  br i1 [[IS_MASTER]], label {{%?}}[[THEN:.+]], label {{%?}}[[EXIT:.+]]
-// CHECK:       [[THEN]]
-// CHECK-NEXT:  invoke {{.*}}void [[FOO]]()
-// CHECK:       call {{.*}}void @__kmpc_end_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
-// CHECK-NEXT:  br label {{%?}}[[EXIT]]
-// CHECK:       [[EXIT]]
+// IRBUILDER:       [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
+// ALL:             [[RES:%.+]] = call {{.*}}i32 @__kmpc_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
+// ALL-NEXT:        [[IS_MASTER:%.+]] = icmp ne i32 [[RES]], 0
+// ALL-NEXT:        br i1 [[IS_MASTER]], label {{%?}}[[THEN:.+]], label {{%?}}[[EXIT:.+]]
+// ALL:             [[THEN]]
+// IRBUILDER-NEXT:  call {{.*}}void [[FOO]]()
+// NORMAL-NEXT:     invoke {{.*}}void [[FOO]]()
+// ALL:             call {{.*}}void @__kmpc_end_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
+// ALL-NEXT:        br label {{%?}}[[EXIT]]
+// ALL:             [[EXIT]]
 #pragma omp master
   foo();
-// CHECK-NOT:   call i32 @__kmpc_master
-// CHECK-NOT:   call void @__kmpc_end_master
+  // ALL-NOT:   call i32 @__kmpc_master
+  // ALL-NOT:   call void @__kmpc_end_master
   return a;
 }
 
-// CHECK-LABEL:      parallel_master
+// ALL-LABEL:      parallel_master
 // TERM_DEBUG-LABEL: parallel_master
 void parallel_master() {
 #pragma omp parallel
 #pragma omp master
   // TERM_DEBUG-NOT: __kmpc_global_thread_num
   // TERM_DEBUG:     call i32 @__kmpc_master({{.+}}), !dbg [[DBG_LOC_START:![0-9]+]]
   // TERM_DEBUG:     invoke void {{.*}}foo{{.*}}()
   // TERM_DEBUG:     unwind label %[[TERM_LPAD:.+]],

llvm/include/llvm/Frontend/OpenMP/OMPConstants.h

 #define LLVM_OPENMP_CONSTANTS_H
 
 #include "llvm/ADT/BitmaskEnum.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace llvm {
 class Type;
 class Module;
+class ArrayType;
 class StructType;
 class PointerType;
 class FunctionType;
 
 namespace omp {
 LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
 
 /// IDs for all OpenMP directives.

 /// generated below. Their names are defined and used in OpenMP/OMPKinds.def.
 /// Here we provide the forward declarations, the initializeTypes function will
 /// provide the values.
 ///
 ///{
 namespace types {
 
 #define OMP_TYPE(VarName, InitValue) extern Type *VarName;
+#define OMP_ARRAY_TYPE(VarName, ElemTy, ArraySize)                             \
+  extern ArrayType *VarName##Ty;                                               \
+  extern PointerType *VarName##PtrTy;
 #define OMP_FUNCTION_TYPE(VarName, IsVarArg, ReturnType, ...)                  \
   extern FunctionType *VarName;                                                \
   extern PointerType *VarName##Ptr;
 #define OMP_STRUCT_TYPE(VarName, StrName, ...)                                 \
   extern StructType *VarName;                                                  \
   extern PointerType *VarName##Ptr;
 #include "llvm/Frontend/OpenMP/OMPKinds.def"
 

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

 //===- IR/OpenMPIRBuilder.h - OpenMP encoding builder for LLVM IR - C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines the OpenMPIRBuilder class and helpers used as a convenient
 // way to create LLVM instructions for OpenMP directives.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_OPENMP_IR_IRBUILDER_H
 #define LLVM_OPENMP_IR_IRBUILDER_H
 
+#include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/IR/IRBuilder.h"
-#include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include "llvm/Support/Allocator.h"
 
 namespace llvm {
 
 /// An interface to create LLVM-IR for OpenMP directives.
 ///
 /// Each OpenMP directive has a corresponding public generator method.
 class OpenMPIRBuilder {
 public:
public:
 
   /// Generator for '#omp cancel'
   ///
   /// \param Loc The location where the directive was encountered.
   /// \param IfCondition The evaluated 'if' clause expression, if any.
   /// \param CanceledDirective The kind of directive that is cancled.
   ///
   /// \returns The insertion point after the barrier.
-  InsertPointTy CreateCancel(const LocationDescription &Loc,
-                              Value *IfCondition,
-                              omp::Directive CanceledDirective);
+  InsertPointTy CreateCancel(const LocationDescription &Loc, Value *IfCondition,
 
   /// Generator for '#omp parallel'
   ///
   /// \param Loc The insert and source location description.
   /// \param BodyGenCB Callback that will generate the region code.
   /// \param PrivCB Callback to copy a given variable (think copy constructor).
   /// \param FiniCB Callback to finalize variable copies.
   /// \param IfCondition The evaluated 'if' clause expression, if any.
   /// \param NumThreads The evaluated 'num_threads' clause expression, if any.
   /// \param ProcBind The value of the 'proc_bind' clause (see ProcBindKind).
   /// \param IsCancellable Flag to indicate a cancellable parallel region.
   ///
   /// \returns The insertion position *after* the parallel.
   IRBuilder<>::InsertPoint
   CreateParallel(const LocationDescription &Loc, BodyGenCallbackTy BodyGenCB,
                  PrivatizeCallbackTy PrivCB, FinalizeCallbackTy FiniCB,
                  Value *IfCondition, Value *NumThreads,
                  omp::ProcBindKind ProcBind, bool IsCancellable);
 
-
   /// Generator for '#omp flush'
   ///
   /// \param Loc The location where the flush directive was encountered
   void CreateFlush(const LocationDescription &Loc);
 
   /// Generator for '#omp taskwait'
   ///
   /// \param Loc The location where the taskwait directive was encountered.
-  void CreateTaskwait(const LocationDescription& Loc);
+  void CreateTaskwait(const LocationDescription &Loc);
 
   /// Generator for '#omp taskyield'
   ///
   /// \param Loc The location where the taskyield directive was encountered.
-  void CreateTaskyield(const LocationDescription& Loc);
+  void CreateTaskyield(const LocationDescription &Loc);
 
   ///}
 
-
 private:
   /// Update the internal location to \p Loc.
   bool updateToLocation(const LocationDescription &Loc) {
     Builder.restoreIP(Loc.IP);
     Builder.SetCurrentDebugLocation(Loc.DL);
     return Loc.IP.getBlock() != nullptr;
   }
 
private:
   /// Map to remember source location strings
   StringMap<Constant *> SrcLocStrMap;
 
   /// Map to remember existing ident_t*.
   DenseMap<std::pair<Constant *, uint64_t>, GlobalVariable *> IdentMap;
 
   /// Helper that contains information about regions we need to outline
   /// during finalization.
   struct OutlineInfo {
     SmallVector<BasicBlock *, 32> Blocks;
     using PostOutlineCBTy = std::function<void(Function &)>;
     PostOutlineCBTy PostOutlineCB;
   };
 
   /// Collection of regions that need to be outlined during finalization.
   SmallVector<OutlineInfo, 16> OutlineInfos;
 
   /// Add a new region that will be outlined later.
   void addOutlineInfo(OutlineInfo &&OI) { OutlineInfos.emplace_back(OI); }
+
+  /// An ordered map of auto-generated variables to their unique names.
+  /// It stores variables with the following names: 1) ".gomp_critical_user_" +
+  /// <critical_section_name> + ".var" for "omp critical" directives; 2)
+  /// <mangled_name_for_global_var> + ".cache." for cache for threadprivate
+  /// variables.
+  StringMap<AssertingVH<Constant>, BumpPtrAllocator> InternalVars;
+
+public:
+  /// Generator for '#omp master'
+  ///
+  /// \param Loc The insert and source location description.
+  /// \param BodyGenCB Callback that will generate the region code.
+  /// \param FiniCB Callback to finalize variable copies.
+  ///
+  /// \returns The insertion position *after* the master.
+  InsertPointTy CreateMaster(const LocationDescription &Loc,
+                             BodyGenCallbackTy BodyGenCB,
+                             FinalizeCallbackTy FiniCB);
+
+  /// Generator for '#omp master'
+  ///
+  /// \param Loc The insert and source location description.
+  /// \param BodyGenCB Callback that will generate the region body code.
+  /// \param FiniCB Callback to finalize variable copies.
+  /// \param CriticalName name of the lock used by the critical directive
+  /// \param HintInst Hint Instruction for hint clause associated with critical
+  ///
+  /// \returns The insertion position *after* the master.
+  InsertPointTy CreateCritical(const LocationDescription &Loc,
+                               BodyGenCallbackTy BodyGenCB,
+                               FinalizeCallbackTy FiniCB,
+                               StringRef CriticalName, Value *HintInst);
+
+private:
+  /// Common interface for generating entry calls for OMP Directives.
+  /// if the directive has a region/body, It will set the insertion
+  /// point to the body
+  ///
+  /// \param OMPD Directive to generate entry blocks for
+  /// \param EntryCall Call to the entry OMP Runtime Function
+  /// \param ExitBB block where the region ends.
+  /// \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.
+  ///
+  /// \return The insertion position in exit block
+  InsertPointTy emitCommonDirectiveEntry(omp::Directive OMPD, Value *EntryCall,
+                                         BasicBlock *ExitBB,
+                                         bool Conditional = false);
+
+  /// 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.
+  ///
+  /// \return The insertion position in exit block
+  InsertPointTy emitCommonDirectiveExit(omp::Directive OMPD,
+                                        InsertPointTy FinIP,
+                                        Instruction *ExitCall,
+                                        bool HasFinalize = true);
+
+  /// 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.
+  ///
+  /// \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);
+
+  /// 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,
+                                           StringRef FirstSeparator,
+                                           StringRef Separator);
+
+  /// Gets (if variable with the given name already exist) or creates
+  /// internal global variable with the specified Name. The created variable has
+  /// linkage CommonLinkage by default and is initialized by null value.
+  /// \param Ty Type of the global variable. If it is exist already the type
+  /// must be the same.
+  /// \param Name Name of the variable.
+  Constant *getOrCreateOMPInternalVariable(Type *Ty, const Twine &Name,
+                                           unsigned AddressSpace = 0);
+
+  /// Returns corresponding lock object for the specified critical region
+  /// name. If the lock object does not exist it is created, otherwise the
+  /// reference to the existing copy is returned.
+  /// \param CriticalName Name of the critical region.
+  ///
+  Value *getOMPCriticalRegionLock(StringRef CriticalName);
 };
 
 } // end namespace llvm
 
 #endif // LLVM_IR_IRBUILDER_H

llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

 __OMP_TYPE(Int8Ptr)
 __OMP_TYPE(Int32Ptr)
 
 #undef __OMP_TYPE
 #undef OMP_TYPE
 
 ///}
 
+/// array types
+///
+///{
+
+#ifndef OMP_ARRAY_TYPE
+#define OMP_ARRAY_TYPE(VarName, ElemTy, ArraySize)
+#endif
+
+#define __OMP_ARRAY_TYPE(VarName, ElemTy, ArraySize)               \
+  OMP_ARRAY_TYPE(VarName, ElemTy, ArraySize)
+
+__OMP_ARRAY_TYPE(KmpCriticalName, Int32, 8)
+
+#undef __OMP_ARRAY_TYPE
+#undef OMP_ARRAY_TYPE
+
+///}
+
 /// Struct and function types
 ///
 ///{
 
 #ifndef OMP_STRUCT_TYPE
 #define OMP_STRUCT_TYPE(VarName, StructName, ...)
 #endif
 

 __OMP_RTL(omp_set_num_threads, false, Void, Int32)
 __OMP_RTL(omp_set_dynamic, false, Void, Int32)
 __OMP_RTL(omp_set_nested, false, Void, Int32)
 __OMP_RTL(omp_set_schedule, false, Void, Int32, Int32)
 __OMP_RTL(omp_set_max_active_levels, false, Void, Int32)
 
 __OMP_RTL(__last, false, Void, )
 
+__OMP_RTL(__kmpc_master, false, Int32, IdentPtr, Int32)
+__OMP_RTL(__kmpc_end_master, false, Void, IdentPtr, Int32)
+__OMP_RTL(__kmpc_critical, false, Void, IdentPtr, Int32, KmpCriticalNamePtrTy)
+__OMP_RTL(__kmpc_critical_with_hint, false, Void, IdentPtr, Int32, KmpCriticalNamePtrTy, Int32)
+__OMP_RTL(__kmpc_end_critical, false, Void, IdentPtr, Int32, KmpCriticalNamePtrTy)
+
 #undef __OMP_RTL
 #undef OMP_RTL
 
 #define EnumAttr(Kind) Attribute::get(Ctx, Attribute::AttrKind::Kind)
 #define AttributeSet(...)                                                      \
   AttributeSet::get(Ctx, ArrayRef<Attribute>({__VA_ARGS__}))
 
 #ifndef OMP_ATTRS_SET

llvm/lib/Frontend/OpenMP/OMPConstants.cpp

 #define OMP_DIRECTIVE(Enum, Str)                                               \
   case Enum:                                                                   \
     return Str;
 #include "llvm/Frontend/OpenMP/OMPKinds.def"
   }
   llvm_unreachable("Invalid OpenMP directive kind");
 }
 
-/// Declarations for LLVM-IR types (simple, function and structure) are
+/// Declarations for LLVM-IR types (simple, array, function and structure) are
 /// generated below. Their names are defined and used in OpenMPKinds.def. Here
 /// we provide the declarations, the initializeTypes function will provide the
 /// values.
 ///
 ///{
-
 #define OMP_TYPE(VarName, InitValue) Type *llvm::omp::types::VarName = nullptr;
+#define OMP_ARRAY_TYPE(VarName, ElemTy, ArraySize)                             \
+  ArrayType *llvm::omp::types::VarName##Ty = nullptr;                          \
+  PointerType *llvm::omp::types::VarName##PtrTy = nullptr;
 #define OMP_FUNCTION_TYPE(VarName, IsVarArg, ReturnType, ...)                  \
   FunctionType *llvm::omp::types::VarName = nullptr;                           \
   PointerType *llvm::omp::types::VarName##Ptr = nullptr;
 #define OMP_STRUCT_TYPE(VarName, StrName, ...)                                 \
   StructType *llvm::omp::types::VarName = nullptr;                             \
   PointerType *llvm::omp::types::VarName##Ptr = nullptr;
 #include "llvm/Frontend/OpenMP/OMPKinds.def"
 
 ///}
 
 void llvm::omp::types::initializeTypes(Module &M) {
   if (Void)
     return;
 
   LLVMContext &Ctx = M.getContext();
   // Create all simple and struct types exposed by the runtime and remember
   // the llvm::PointerTypes of them for easy access later.
   StructType *T;
 #define OMP_TYPE(VarName, InitValue) VarName = InitValue;
+#define OMP_ARRAY_TYPE(VarName, ElemTy, ArraySize)                             \
+  VarName##Ty = ArrayType::get(ElemTy, ArraySize);                             \
+  VarName##PtrTy = PointerType::getUnqual(VarName##Ty);
 #define OMP_FUNCTION_TYPE(VarName, IsVarArg, ReturnType, ...)                  \
   VarName = FunctionType::get(ReturnType, {__VA_ARGS__}, IsVarArg);            \
   VarName##Ptr = PointerType::getUnqual(VarName);
 #define OMP_STRUCT_TYPE(VarName, StructName, ...)                              \
   T = M.getTypeByName(StructName);                                             \
   if (!T)                                                                      \
     T = StructType::create(Ctx, {__VA_ARGS__}, StructName);                    \
   VarName = T;                                                                 \

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/DebugInfo.h"
-#include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/MDBuilder.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/CodeExtractor.h"
 
 #include <sstream>
 
 #define DEBUG_TYPE "openmp-ir-builder"
IRBuilder<>::InsertPoint OpenMPIRBuilder::CreateParallel(
   addOutlineInfo(std::move(OI));
 
   InsertPointTy AfterIP(UI->getParent(), UI->getParent()->end());
   UI->eraseFromParent();
 
   return AfterIP;
 }
 
-void OpenMPIRBuilder::emitFlush(const LocationDescription &Loc)
-{
+void OpenMPIRBuilder::emitFlush(const LocationDescription &Loc) {
   // Build call void __kmpc_flush(ident_t *loc)
   Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
   Value *Args[] = {getOrCreateIdent(SrcLocStr)};
 
   Builder.CreateCall(getOrCreateRuntimeFunction(OMPRTL___kmpc_flush), Args);
 }
 
-void OpenMPIRBuilder::CreateFlush(const LocationDescription &Loc)
-{
+void OpenMPIRBuilder::CreateFlush(const LocationDescription &Loc) {
   if (!updateToLocation(Loc))
     return;
   emitFlush(Loc);
 }
 
 void OpenMPIRBuilder::emitTaskwaitImpl(const LocationDescription &Loc) {
   // Build call kmp_int32 __kmpc_omp_taskwait(ident_t *loc, kmp_int32
   // global_tid);
   Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
   Value *Ident = getOrCreateIdent(SrcLocStr);
Builder.CreateCall(getOrCreateRuntimeFunction(OMPRTL___kmpc_omp_taskyield),
                      Args);
 }
 
 void OpenMPIRBuilder::CreateTaskyield(const LocationDescription &Loc) {
   if (!updateToLocation(Loc))
     return;
   emitTaskyieldImpl(Loc);
 }
+
+OpenMPIRBuilder::InsertPointTy
+OpenMPIRBuilder::CreateMaster(const LocationDescription &Loc,
+                              BodyGenCallbackTy BodyGenCB,
+                              FinalizeCallbackTy FiniCB) {
+
+  if (!updateToLocation(Loc))
+    return Loc.IP;
+
+  Directive OMPD = Directive::OMPD_master;
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Value *ThreadId = getOrCreateThreadID(Ident);
+  Value *Args[] = {Ident, ThreadId};
+
+  Function *EntryRTLFn = getOrCreateRuntimeFunction(OMPRTL___kmpc_master);
+  Instruction *EntryCall = Builder.CreateCall(EntryRTLFn, Args);
+
+  Function *ExitRTLFn = getOrCreateRuntimeFunction(OMPRTL___kmpc_end_master);
+  Instruction *ExitCall = Builder.CreateCall(ExitRTLFn, Args);
+
+  return EmitOMPInlinedRegion(OMPD, EntryCall, ExitCall, BodyGenCB, FiniCB,
+                              /*Conditional*/ true, /*hasFinalize*/ true);
+}
+
+OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::CreateCritical(
+    const LocationDescription &Loc, BodyGenCallbackTy BodyGenCB,
+    FinalizeCallbackTy FiniCB, StringRef CriticalName, Value *HintInst) {
+
+  if (!updateToLocation(Loc))
+    return Loc.IP;
+
+  Directive OMPD = Directive::OMPD_critical;
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Value *ThreadId = getOrCreateThreadID(Ident);
+  Value *LockVar = getOMPCriticalRegionLock(CriticalName);
+  Value *Args[] = {Ident, ThreadId, LockVar};
+
+  SmallVector<llvm::Value *, 4> EnterArgs(std::begin(Args), std::end(Args));
+  Function *RTFn = nullptr;
+  if (HintInst) {
+    // Add Hint to entry Args and create call
+    EnterArgs.push_back(HintInst);
+    RTFn = getOrCreateRuntimeFunction(OMPRTL___kmpc_critical_with_hint);
+  } else {
+    RTFn = getOrCreateRuntimeFunction(OMPRTL___kmpc_critical);
+  }
+  Instruction *EntryCall = Builder.CreateCall(RTFn, EnterArgs);
+
+  Function *ExitRTLFn = getOrCreateRuntimeFunction(OMPRTL___kmpc_end_critical);
+  Instruction *ExitCall = Builder.CreateCall(ExitRTLFn, Args);
+
+  return EmitOMPInlinedRegion(OMPD, EntryCall, ExitCall, BodyGenCB, FiniCB,
+                              /*Conditional*/ false, /*hasFinalize*/ true);
+}
+
+OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::EmitOMPInlinedRegion(
+    Directive OMPD, Instruction *EntryCall, Instruction *ExitCall,
+    BodyGenCallbackTy BodyGenCB, FinalizeCallbackTy FiniCB, bool Conditional,
+    bool HasFinalize) {
+
+  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);
+
+  // 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);
+    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 &&
+         "Unexpected Insertion point location!");
+  if (!Conditional && SkipEmittingRegion) {
+    ExitBB->eraseFromParent();
+    Builder.ClearInsertionPoint();
+  } else {
+    auto merged = MergeBlockIntoPredecessor(ExitBB);
+    BasicBlock *ExitPredBB = SplitPos->getParent();
+    auto InsertBB = merged ? ExitPredBB : ExitBB;
+    if (!isa_and_nonnull<BranchInst>(SplitPos))
+      SplitPos->eraseFromParent();
+    Builder.SetInsertPoint(InsertBB);
+  }
+
+  return Builder.saveIP();
+}
+
+OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::emitCommonDirectiveEntry(
+    Directive OMPD, Value *EntryCall, BasicBlock *ExitBB, bool Conditional) {
+
+  // if nothing to do, Return current insertion point.
+  if (!Conditional)
+    return Builder.saveIP();
+
+  BasicBlock *EntryBB = Builder.GetInsertBlock();
+  Value *CallBool = Builder.CreateIsNotNull(EntryCall);
+  auto *ThenBB = BasicBlock::Create(M.getContext(), "omp_region.body");
+  auto *UI = new UnreachableInst(Builder.getContext(), ThenBB);
+
+  // Emit thenBB and set the Builder's insertion point there for
+  // body generation next. Place the block after the current block.
+  Function *CurFn = EntryBB->getParent();
+  CurFn->getBasicBlockList().insertAfter(EntryBB->getIterator(), ThenBB);
+
+  // Move Entry branch to end of ThenBB, and replace with conditional
+  // branch (If-stmt)
+  Instruction *EntryBBTI = EntryBB->getTerminator();
+  Builder.CreateCondBr(CallBool, ThenBB, ExitBB);
+  EntryBBTI->removeFromParent();
+  Builder.SetInsertPoint(UI);
+  Builder.Insert(EntryBBTI);
+  UI->eraseFromParent();
+  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) {
+
+  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);
+  }
+
+  // place the Exitcall as last instruction before Finalization block terminator
+  ExitCall->removeFromParent();
+  Builder.Insert(ExitCall);
+
+  return IRBuilder<>::InsertPoint(ExitCall->getParent(),
+                                  ExitCall->getIterator());
+}
+
+std::string OpenMPIRBuilder::getNameWithSeparators(ArrayRef<StringRef> Parts,
+                                                   StringRef FirstSeparator,
+                                                   StringRef Separator) {
+  SmallString<128> Buffer;
+  llvm::raw_svector_ostream OS(Buffer);
+  StringRef Sep = FirstSeparator;
+  for (StringRef Part : Parts) {
+    OS << Sep << Part;
+    Sep = Separator;
+  }
+  return OS.str().str();
+}
+
+Constant *OpenMPIRBuilder::getOrCreateOMPInternalVariable(
+    llvm::Type *Ty, const llvm::Twine &Name, unsigned AddressSpace) {
+  // TODO: Replace the twine arg with stringref to get rid of the conversion
+  // logic. However This is taken from current implementation in clang as is.
+  // Since this method is used in many places exclusively for OMP internal use
+  // we will keep it as is for temporarily until we move all users to the
+  // builder and then, if possible, fix it everywhere in one go.
+  SmallString<256> Buffer;
+  llvm::raw_svector_ostream Out(Buffer);
+  Out << Name;
+  StringRef RuntimeName = Out.str();
+  auto &Elem = *InternalVars.try_emplace(RuntimeName, nullptr).first;
+  if (Elem.second) {
+    assert(Elem.second->getType()->getPointerElementType() == Ty &&
+           "OMP internal variable has different type than requested");
+  } else {
+    // TODO: investigate the appropriate linkage type used for the global
+    // variable for possibly changing that to internal or private, or maybe
+    // create different versions of the function for different OMP internal
+    // variables.
+    Elem.second = new llvm::GlobalVariable(
+        M, Ty, /*IsConstant*/ false, llvm::GlobalValue::CommonLinkage,
+        llvm::Constant::getNullValue(Ty), Elem.first(),
+        /*InsertBefore=*/nullptr, llvm::GlobalValue::NotThreadLocal,
+        AddressSpace);
+  }
+
+  return Elem.second;
+}
+
+Value *OpenMPIRBuilder::getOMPCriticalRegionLock(StringRef CriticalName) {
+  std::string Prefix = Twine("gomp_critical_user_", CriticalName).str();
+  std::string Name = getNameWithSeparators({Prefix, "var"}, ".", ".");
+  return getOrCreateOMPInternalVariable(KmpCriticalNameTy, Name);
+}

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

if (!isa<ReturnInst>(ExitBB->front())) {
       ASSERT_TRUE(isa<BranchInst>(ExitBB->front()));
       ASSERT_EQ(cast<BranchInst>(ExitBB->front()).getNumSuccessors(), 1U);
       ASSERT_TRUE(isa<ReturnInst>(
           cast<BranchInst>(ExitBB->front()).getSuccessor(0)->front()));
     }
   }
 }
 
+TEST_F(OpenMPIRBuilderTest, MasterDirective) {
+  using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+
+  AllocaInst *PrivAI = nullptr;
+
+  BasicBlock *EntryBB = nullptr;
+  BasicBlock *ExitBB = nullptr;
+  BasicBlock *ThenBB = nullptr;
+
+  auto BodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+                       BasicBlock &FiniBB) {
+    if (AllocaIP.isSet())
+      Builder.restoreIP(AllocaIP);
+    else
+      Builder.SetInsertPoint(&*(F->getEntryBlock().getFirstInsertionPt()));
+    PrivAI = Builder.CreateAlloca(F->arg_begin()->getType());
+    Builder.CreateStore(F->arg_begin(), PrivAI);
+
+    llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
+    llvm::Instruction *CodeGenIPInst = &*CodeGenIP.getPoint();
+    EXPECT_EQ(CodeGenIPBB->getTerminator(), CodeGenIPInst);
+
+    Builder.restoreIP(CodeGenIP);
+
+    // collect some info for checks later
+    ExitBB = FiniBB.getUniqueSuccessor();
+    ThenBB = Builder.GetInsertBlock();
+    EntryBB = ThenBB->getUniquePredecessor();
+
+    // simple instructions for body
+    Value *PrivLoad = Builder.CreateLoad(PrivAI, "local.use");
+    Builder.CreateICmpNE(F->arg_begin(), PrivLoad);
+  };
+
+  auto FiniCB = [&](InsertPointTy IP) {
+    BasicBlock *IPBB = IP.getBlock();
+    EXPECT_NE(IPBB->end(), IP.getPoint());
+  };
+
+  Builder.restoreIP(OMPBuilder.CreateMaster(Builder, BodyGenCB, FiniCB));
+  Value *EntryBBTI = EntryBB->getTerminator();
+  EXPECT_NE(EntryBBTI, nullptr);
+  EXPECT_TRUE(isa<BranchInst>(EntryBBTI));
+  BranchInst *EntryBr = cast<BranchInst>(EntryBB->getTerminator());
+  EXPECT_TRUE(EntryBr->isConditional());
+  EXPECT_EQ(EntryBr->getSuccessor(0), ThenBB);
+  EXPECT_EQ(ThenBB->getUniqueSuccessor(), ExitBB);
+  EXPECT_EQ(EntryBr->getSuccessor(1), ExitBB);
+
+  CmpInst *CondInst = cast<CmpInst>(EntryBr->getCondition());
+  EXPECT_TRUE(isa<CallInst>(CondInst->getOperand(0)));
+
+  CallInst *MasterEntryCI = cast<CallInst>(CondInst->getOperand(0));
+  EXPECT_EQ(MasterEntryCI->getNumArgOperands(), 2U);
+  EXPECT_EQ(MasterEntryCI->getCalledFunction()->getName(), "__kmpc_master");
+  EXPECT_TRUE(isa<GlobalVariable>(MasterEntryCI->getArgOperand(0)));
+
+  CallInst *MasterEndCI = nullptr;
+  for (auto &FI : *ThenBB) {
+    Instruction *cur = &FI;
+    if (isa<CallInst>(cur)) {
+      MasterEndCI = cast<CallInst>(cur);
+      if (MasterEndCI->getCalledFunction()->getName() == "__kmpc_end_master")
+        break;
+      MasterEndCI = nullptr;
+    }
+  }
+  EXPECT_NE(MasterEndCI, nullptr);
+  EXPECT_EQ(MasterEndCI->getNumArgOperands(), 2U);
+  EXPECT_TRUE(isa<GlobalVariable>(MasterEndCI->getArgOperand(0)));
+  EXPECT_EQ(MasterEndCI->getArgOperand(1), MasterEntryCI->getArgOperand(1));
+}
+
+TEST_F(OpenMPIRBuilderTest, CriticalDirective) {
+  using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+
+  AllocaInst *PrivAI = Builder.CreateAlloca(F->arg_begin()->getType());
+
+  BasicBlock *EntryBB = nullptr;
+
+  auto BodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+                       BasicBlock &FiniBB) {
+    // collect some info for checks later
+    EntryBB = FiniBB.getUniquePredecessor();
+
+    // actual start for bodyCB
+    llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
+    llvm::Instruction *CodeGenIPInst = &*CodeGenIP.getPoint();
+    EXPECT_EQ(CodeGenIPBB->getTerminator(), CodeGenIPInst);
+    EXPECT_EQ(EntryBB, CodeGenIPBB);
+
+    // body begin
+    Builder.restoreIP(CodeGenIP);
+    Builder.CreateStore(F->arg_begin(), PrivAI);
+    Value *PrivLoad = Builder.CreateLoad(PrivAI, "local.use");
+    Builder.CreateICmpNE(F->arg_begin(), PrivLoad);
+  };
+
+  auto FiniCB = [&](InsertPointTy IP) {
+    BasicBlock *IPBB = IP.getBlock();
+    EXPECT_NE(IPBB->end(), IP.getPoint());
+  };
+
+  Builder.restoreIP(OMPBuilder.CreateCritical(Builder, BodyGenCB, FiniCB,
+                                              "testCRT", nullptr));
+
+  Value *EntryBBTI = EntryBB->getTerminator();
+  EXPECT_EQ(EntryBBTI, nullptr);
+
+  CallInst *CriticalEntryCI = nullptr;
+  for (auto &EI : *EntryBB) {
+    Instruction *cur = &EI;
+    if (isa<CallInst>(cur)) {
+      CriticalEntryCI = cast<CallInst>(cur);
+      if (CriticalEntryCI->getCalledFunction()->getName() == "__kmpc_critical")
+        break;
+      CriticalEntryCI = nullptr;
+    }
+  }
+  EXPECT_NE(CriticalEntryCI, nullptr);
+  EXPECT_EQ(CriticalEntryCI->getNumArgOperands(), 3U);
+  EXPECT_EQ(CriticalEntryCI->getCalledFunction()->getName(), "__kmpc_critical");
+  EXPECT_TRUE(isa<GlobalVariable>(CriticalEntryCI->getArgOperand(0)));
+
+  CallInst *CriticalEndCI = nullptr;
+  for (auto &FI : *EntryBB) {
+    Instruction *cur = &FI;
+    if (isa<CallInst>(cur)) {
+      CriticalEndCI = cast<CallInst>(cur);
+      if (CriticalEndCI->getCalledFunction()->getName() ==
+          "__kmpc_end_critical")
+        break;
+      CriticalEndCI = nullptr;
+    }
+  }
+  EXPECT_NE(CriticalEndCI, nullptr);
+  EXPECT_EQ(CriticalEndCI->getNumArgOperands(), 3U);
+  EXPECT_TRUE(isa<GlobalVariable>(CriticalEndCI->getArgOperand(0)));
+  EXPECT_EQ(CriticalEndCI->getArgOperand(1), CriticalEntryCI->getArgOperand(1));
+  PointerType *CriticalNamePtrTy =
+      PointerType::getUnqual(ArrayType::get(Type::getInt32Ty(Ctx), 8));
+  EXPECT_EQ(CriticalEndCI->getArgOperand(2), CriticalEntryCI->getArgOperand(2));
+  EXPECT_EQ(CriticalEndCI->getArgOperand(2)->getType(), CriticalNamePtrTy);
+}
+
 } // namespace