[clang] ba3f863 - [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class
Johannes Doerfert via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 19 12:12:12 PST 2020
Author: Fady Ghanim
Date: 2020-02-19T14:11:17-06:00
New Revision: ba3f863dfb9c5f9bf5e6fdca2198b609df3b7761
URL: https://github.com/llvm/llvm-project/commit/ba3f863dfb9c5f9bf5e6fdca2198b609df3b7761
DIFF: https://github.com/llvm/llvm-project/commit/ba3f863dfb9c5f9bf5e6fdca2198b609df3b7761.diff
LOG: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class
This patch introduces a new helper class `OMPBuilderCBHelpers`,
which will contain all reusable C/C++ language specific function-
alities required by the `OMPIRBuilder`.
Initially, this helper class contains the body and finalization
codegen functionalities implemented using callbacks which were
moved here for reusability among the different directives
implemented in the `OMPIRBuilder`, along with RAIIs for preserving
state prior to emitting outlined and/or inlined OpenMP regions.
In the future this helper class will also contain all the different
call backs required by OpenMP clauses/variable privatization.
Reviewed By: jdoerfert
Differential Revision: https://reviews.llvm.org/D74562
Added:
Modified:
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/test/OpenMP/cancel_codegen.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index d7115e00836c..bcd2d0635caf 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1450,15 +1450,7 @@ void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
// The cleanup callback that finalizes all variabels at the given location,
// thus calls destructors etc.
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->splitBasicBlock(IP.getPoint());
- IPBB->getTerminator()->eraseFromParent();
- Builder.SetInsertPoint(IPBB);
- CodeGenFunction::JumpDest Dest = getJumpDestInCurrentScope(DestBB);
- EmitBranchThroughCleanup(Dest);
+ OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
};
// Privatization callback that performs appropriate action for
@@ -1480,25 +1472,10 @@ void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
auto BodyGenCB = [ParallelRegionBodyStmt,
this](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
llvm::BasicBlock &ContinuationBB) {
- auto OldAllocaIP = AllocaInsertPt;
- AllocaInsertPt = &*AllocaIP.getPoint();
-
- auto OldReturnBlock = ReturnBlock;
- ReturnBlock = getJumpDestInCurrentScope(&ContinuationBB);
-
- llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
- CodeGenIPBB->splitBasicBlock(CodeGenIP.getPoint());
- llvm::Instruction *CodeGenIPBBTI = CodeGenIPBB->getTerminator();
- CodeGenIPBBTI->removeFromParent();
-
- Builder.SetInsertPoint(CodeGenIPBB);
-
- EmitStmt(ParallelRegionBodyStmt);
-
- Builder.Insert(CodeGenIPBBTI);
-
- AllocaInsertPt = OldAllocaIP;
- ReturnBlock = OldReturnBlock;
+ OMPBuilderCBHelpers::OutlinedRegionBodyRAII ORB(*this, AllocaIP,
+ ContinuationBB);
+ OMPBuilderCBHelpers::EmitOMPRegionBody(*this, ParallelRegionBodyStmt,
+ CodeGenIP, ContinuationBB);
};
CGCapturedStmtInfo CGSI(*CS, CR_OpenMP);
@@ -3152,55 +3129,18 @@ void CodeGenFunction::EmitOMPMasterDirective(const OMPMasterDirective &S) {
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);
+ OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
};
- // 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;
+ OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP, FiniBB);
+ OMPBuilderCBHelpers::EmitOMPRegionBody(*this, MasterRegionBodyStmt,
+ CodeGenIP, FiniBB);
};
+
CGCapturedStmtInfo CGSI(*CS, CR_OpenMP);
CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
Builder.restoreIP(OMPBuilder->CreateMaster(Builder, BodyGenCB, FiniCB));
@@ -3229,53 +3169,16 @@ void CodeGenFunction::EmitOMPCriticalDirective(const OMPCriticalDirective &S) {
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);
+ OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
};
- // 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;
+ OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP, FiniBB);
+ OMPBuilderCBHelpers::EmitOMPRegionBody(*this, CriticalRegionBodyStmt,
+ CodeGenIP, FiniBB);
};
CGCapturedStmtInfo CGSI(*CS, CR_OpenMP);
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 1e3244d807da..c3b2ae0fda09 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -36,6 +36,7 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
#include "llvm/IR/ValueHandle.h"
#include "llvm/Support/Debug.h"
#include "llvm/Transforms/Utils/SanitizerStats.h"
@@ -255,6 +256,114 @@ class CodeGenFunction : public CodeGenTypeCache {
unsigned Index;
};
+ // Helper class for the OpenMP IR Builder. Allows reusability of code used for
+ // region body, and finalization codegen callbacks. This will class will also
+ // contain privatization functions used by the privatization call backs
+ struct OMPBuilderCBHelpers {
+
+ using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+
+ /// Emit the Finalization for an OMP region
+ /// \param CGF The Codegen function this belongs to
+ /// \param IP Insertion point for generating the finalization code.
+ static void FinalizeOMPRegion(CodeGenFunction &CGF, InsertPointTy IP) {
+ CGBuilderTy::InsertPointGuard IPG(CGF.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();
+ CGF.Builder.SetInsertPoint(IPBB);
+ CodeGenFunction::JumpDest Dest = CGF.getJumpDestInCurrentScope(DestBB);
+ CGF.EmitBranchThroughCleanup(Dest);
+ }
+
+ /// Emit the body of an OMP region
+ /// \param CGF The Codegen function this belongs to
+ /// \param RegionBodyStmt The body statement for the OpenMP region being
+ /// generated
+ /// \param CodeGenIP Insertion point for generating the body code.
+ /// \param FiniBB The finalization basic block
+ static void EmitOMPRegionBody(CodeGenFunction &CGF,
+ const Stmt *RegionBodyStmt,
+ InsertPointTy CodeGenIP,
+ llvm::BasicBlock &FiniBB) {
+ llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
+ if (llvm::Instruction *CodeGenIPBBTI = CodeGenIPBB->getTerminator())
+ CodeGenIPBBTI->eraseFromParent();
+
+ CGF.Builder.SetInsertPoint(CodeGenIPBB);
+
+ CGF.EmitStmt(RegionBodyStmt);
+
+ if (CGF.Builder.saveIP().isSet())
+ CGF.Builder.CreateBr(&FiniBB);
+ }
+
+ /// RAII for preserving necessary info during Outlined region body codegen.
+ class OutlinedRegionBodyRAII {
+
+ llvm::AssertingVH<llvm::Instruction> OldAllocaIP;
+ CodeGenFunction::JumpDest OldReturnBlock;
+ CodeGenFunction &CGF;
+
+ public:
+ OutlinedRegionBodyRAII(CodeGenFunction &cgf, InsertPointTy &AllocaIP,
+ llvm::BasicBlock &RetBB)
+ : CGF(cgf) {
+ assert(AllocaIP.isSet() &&
+ "Must specify Insertion point for allocas of outlined function");
+ OldAllocaIP = CGF.AllocaInsertPt;
+ CGF.AllocaInsertPt = &*AllocaIP.getPoint();
+
+ OldReturnBlock = CGF.ReturnBlock;
+ CGF.ReturnBlock = CGF.getJumpDestInCurrentScope(&RetBB);
+ }
+
+ ~OutlinedRegionBodyRAII() {
+ CGF.AllocaInsertPt = OldAllocaIP;
+ CGF.ReturnBlock = OldReturnBlock;
+ }
+ };
+
+ /// RAII for preserving necessary info during inlined region body codegen.
+ class InlinedRegionBodyRAII {
+
+ llvm::AssertingVH<llvm::Instruction> OldAllocaIP;
+ CodeGenFunction &CGF;
+
+ public:
+ InlinedRegionBodyRAII(CodeGenFunction &cgf, InsertPointTy &AllocaIP,
+ llvm::BasicBlock &FiniBB)
+ : CGF(cgf) {
+ // 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() ||
+ CGF.AllocaInsertPt->getParent() == AllocaIP.getBlock()) &&
+ "Insertion point should be in the entry block of containing "
+ "function!");
+ OldAllocaIP = CGF.AllocaInsertPt;
+ if (AllocaIP.isSet())
+ CGF.AllocaInsertPt = &*AllocaIP.getPoint();
+
+ // TODO: Remove the call, after making sure the counter is not used by
+ // the EHStack.
+ // Since this is an inlined region, it should not modify the
+ // ReturnBlock, and should reuse the one for the enclosing outlined
+ // region. So, the JumpDest being return by the function is discarded
+ (void)CGF.getJumpDestInCurrentScope(&FiniBB);
+ }
+
+ ~InlinedRegionBodyRAII() { CGF.AllocaInsertPt = OldAllocaIP; }
+ };
+ };
+
CodeGenModule &CGM; // Per-module state.
const TargetInfo &Target;
diff --git a/clang/test/OpenMP/cancel_codegen.cpp b/clang/test/OpenMP/cancel_codegen.cpp
index 8402f0fef0df..b7d1cea56721 100644
--- a/clang/test/OpenMP/cancel_codegen.cpp
+++ b/clang/test/OpenMP/cancel_codegen.cpp
@@ -193,11 +193,9 @@ for (int i = 0; i < argc; ++i) {
// IRBUILDER: br i1 [[CMP]], label %[[CONTINUE:[^,].+]], label %[[EXIT:.+]]
// IRBUILDER: [[EXIT]]
// IRBUILDER: br label %[[EXIT2:.+]]
-// IRBUILDER: [[EXIT2]]
-// IRBUILDER: br label %[[EXIT3:.+]]
// IRBUILDER: [[CONTINUE]]
// IRBUILDER: br label %[[ELSE:.+]]
-// IRBUILDER: [[EXIT3]]
+// IRBUILDER: [[EXIT2]]
// IRBUILDER: br label %[[RETURN]]
#endif
More information about the cfe-commits
mailing list