[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class
This revision was automatically updated to reflect the committed changes.
Closed by commit rGba3f863dfb9c: [OpenMP][OMPIRBuilder] Introducing the
`OMPBuilderCBHelpers` helper class (authored by fghanim, committed by
jdoerfert).
Changed prior to commit:
https://reviews.llvm.org/D74562?vs=245227&id=245486#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74562/new/
https://reviews.llvm.org/D74562
Files:
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/test/OpenMP/cancel_codegen.cpp
Index: clang/test/OpenMP/cancel_codegen.cpp
===
--- clang/test/OpenMP/cancel_codegen.cpp
+++ clang/test/OpenMP/cancel_codegen.cpp
@@ -193,11 +193,9 @@
// 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
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ 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 @@
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 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 OldAllocaIP;
+ CodeGenFunction &CGF;
+
+public:
+ InlinedRegionBodyRAII(CodeGenFunction &cgf, InsertPointTy &AllocaIP,
+
[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class
fghanim added a comment. I am done updating this patch. I still don't have commit access, I'd appreciate it if you'd commit this for me when you get a chance. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74562/new/ https://reviews.llvm.org/D74562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class
fghanim updated this revision to Diff 245227.
fghanim added a comment.
Marking a call void
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74562/new/
https://reviews.llvm.org/D74562
Files:
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/test/OpenMP/cancel_codegen.cpp
Index: clang/test/OpenMP/cancel_codegen.cpp
===
--- clang/test/OpenMP/cancel_codegen.cpp
+++ clang/test/OpenMP/cancel_codegen.cpp
@@ -192,9 +192,7 @@
// IRBUILDER: [[CMP:%.+]] = icmp eq i32 [[RES]], 0
// IRBUILDER: br i1 [[CMP]], label %[[CONTINUE:[^,].+]], label %[[EXIT:.+]]
// IRBUILDER: [[EXIT]]
-// IRBUILDER: br label %[[EXIT2:.+]]
-// IRBUILDER: [[EXIT2]]
-// IRBUILDER: br label %[[RETURN]]
+// IRBUILDER: br label %[[RETURN:.+]]
// IRBUILDER: [[CONTINUE]]
// IRBUILDER: br label %[[ELSE:.+]]
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ 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 @@
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 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 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 re
[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class
fghanim updated this revision to Diff 245151.
fghanim added a comment.
addressing review comments - Adding a comment to explain minor change.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74562/new/
https://reviews.llvm.org/D74562
Files:
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/test/OpenMP/cancel_codegen.cpp
Index: clang/test/OpenMP/cancel_codegen.cpp
===
--- clang/test/OpenMP/cancel_codegen.cpp
+++ clang/test/OpenMP/cancel_codegen.cpp
@@ -192,9 +192,7 @@
// IRBUILDER: [[CMP:%.+]] = icmp eq i32 [[RES]], 0
// IRBUILDER: br i1 [[CMP]], label %[[CONTINUE:[^,].+]], label %[[EXIT:.+]]
// IRBUILDER: [[EXIT]]
-// IRBUILDER: br label %[[EXIT2:.+]]
-// IRBUILDER: [[EXIT2]]
-// IRBUILDER: br label %[[RETURN]]
+// IRBUILDER: br label %[[RETURN:.+]]
// IRBUILDER: [[CONTINUE]]
// IRBUILDER: br label %[[ELSE:.+]]
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ 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 @@
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 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 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
[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class
jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM assuming the comment below is addressed. Comment at: clang/lib/CodeGen/CodeGenFunction.h:354 + +FinilizationBlock = CGF.getJumpDestInCurrentScope(&FiniBB); + } fghanim wrote: > jdoerfert wrote: > > Don't we have to set/reset the `CGF.ReturnBlock` ? If not, why do we need > > `FinilizationBlock` here? > Yes, and we still do for outlined regions (check `OutlinedRegionBodyRAII` > ctor above). > > This is for inlined regions, and those shouldn't change the > `CGF.ReturnBlock`. It should re-use the `CGF.ReturnBlock` for the enclosing > outlined region. > The only reason I kept it, is because `CGF.getJumpDestInCurrentScope()` was > updating a counter that seemed to me to relate to `EHStack`, and I didn't > want to remove it without investigating first. I see. Maybe remove the `FinilizationBlock` member though. Call `getJumpDestInCurrentScope` and add a comment (TODO) that explains this, e.g., says it has to be investigated further. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74562/new/ https://reviews.llvm.org/D74562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class
fghanim added a comment. Regarding the third comment (which was removed for some reason), This new update should fix that Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3134 // TODO: Replace with a generic helper function for emitting body auto BodyGenCB = [MasterRegionBodyStmt, this](InsertPointTy AllocaIP, jdoerfert wrote: > These TODOs are now obsolete ;) We need to split our patches to keep them small :p Comment at: clang/lib/CodeGen/CodeGenFunction.h:354 + +FinilizationBlock = CGF.getJumpDestInCurrentScope(&FiniBB); + } jdoerfert wrote: > Don't we have to set/reset the `CGF.ReturnBlock` ? If not, why do we need > `FinilizationBlock` here? Yes, and we still do for outlined regions (check `OutlinedRegionBodyRAII` ctor above). This is for inlined regions, and those shouldn't change the `CGF.ReturnBlock`. It should re-use the `CGF.ReturnBlock` for the enclosing outlined region. The only reason I kept it, is because `CGF.getJumpDestInCurrentScope()` was updating a counter that seemed to me to relate to `EHStack`, and I didn't want to remove it without investigating first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74562/new/ https://reviews.llvm.org/D74562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class
fghanim updated this revision to Diff 245014.
fghanim marked 2 inline comments as done.
fghanim added a comment.
Fixed bug where variables where still being allocated in original function
entry block
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74562/new/
https://reviews.llvm.org/D74562
Files:
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/test/OpenMP/cancel_codegen.cpp
Index: clang/test/OpenMP/cancel_codegen.cpp
===
--- clang/test/OpenMP/cancel_codegen.cpp
+++ clang/test/OpenMP/cancel_codegen.cpp
@@ -192,9 +192,7 @@
// IRBUILDER: [[CMP:%.+]] = icmp eq i32 [[RES]], 0
// IRBUILDER: br i1 [[CMP]], label %[[CONTINUE:[^,].+]], label %[[EXIT:.+]]
// IRBUILDER: [[EXIT]]
-// IRBUILDER: br label %[[EXIT2:.+]]
-// IRBUILDER: [[EXIT2]]
-// IRBUILDER: br label %[[RETURN]]
+// IRBUILDER: br label %[[RETURN:.+]]
// IRBUILDER: [[CONTINUE]]
// IRBUILDER: br label %[[ELSE:.+]]
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ 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,110 @@
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 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 OldAllocaIP;
+ CodeGenFunction::JumpDest FinilizationBlock;
+ CodeGenFunction &CGF;
+
+public:
+ InlinedRegionBodyRAII(CodeGenFunction &cgf, InsertPointTy &AllocaIP,
+llvm::BasicBlock &FiniBB)
+ : CGF(c
[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class
jdoerfert added a comment.
This is great, thanks a lot! I only have two comments where I am not sure I
understand the code/change.
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3134
// TODO: Replace with a generic helper function for emitting body
auto BodyGenCB = [MasterRegionBodyStmt, this](InsertPointTy AllocaIP,
These TODOs are now obsolete ;)
Comment at: clang/lib/CodeGen/CodeGenFunction.h:354
+
+FinilizationBlock = CGF.getJumpDestInCurrentScope(&FiniBB);
+ }
Don't we have to set/reset the `CGF.ReturnBlock` ? If not, why do we need
`FinilizationBlock` here?
Comment at: clang/test/OpenMP/parallel_codegen.cpp:143
// CHECK-DEBUG: define internal void [[OMP_OUTLINED_DEBUG:@.+]](i32*
noalias %.global_tid., i32* noalias %.bound_tid., i8***
dereferenceable({{4|8}}) %argc, i64 %{{.+}})
-// IRBUILDER-DEBUG: define internal void [[OMP_OUTLINED_DEBUG:@.+]](i32*
noalias %{{.*}}, i32* noalias %{{.*}}, i8*** [[ARGC_REF:%.*]], i64 %{{.+}})
+// IRBUILDER-DEBUG: define internal void [[OMP_OUTLINED_DEBUG:@.+]](i32*
noalias %{{.*}}, i32* noalias %{{.*}}, i8*** [[ARGC_REF:%.*]], double**
[[VAR]], i64 %{{.+}})
// CHECK-DEBUG: store i8*** %argc, i8 [[ARGC_PTR_ADDR:%.+]],
Do you know why this changed? Is this variable used in the parallel region?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74562/new/
https://reviews.llvm.org/D74562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class
fghanim created this revision.
fghanim added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, guansong.
Herald added a project: clang.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D74562
Files:
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/test/OpenMP/cancel_codegen.cpp
clang/test/OpenMP/parallel_codegen.cpp
Index: clang/test/OpenMP/parallel_codegen.cpp
===
--- clang/test/OpenMP/parallel_codegen.cpp
+++ clang/test/OpenMP/parallel_codegen.cpp
@@ -112,7 +112,7 @@
// ALL: define linkonce_odr {{[a-z\_\b]*[ ]?i32}} [[TMAIN]](i8** %argc)
// ALL: store i8** %argc, i8*** [[ARGC_ADDR:%.+]],
// CHECK: call {{.*}}void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* [[DEF_LOC_2]], i32 2, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i8***, i{{64|32}})* [[OMP_OUTLINED:@.+]] to void (i32*, i32*, ...)*), i8*** [[ARGC_ADDR]], i{{64|32}} %{{.+}})
-// IRBUILDER: call {{.*}}void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* [[DEF_LOC_2]], i32 2, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i8***, i{{64|32}})* [[OMP_OUTLINED:@.+]] to void (i32*, i32*, ...)*), i8*** [[ARGC_ADDR]], i{{64|32}} %{{.+}})
+// IRBUILDER: call {{.*}}void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* [[DEF_LOC_2]], i32 3, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i8***, double**, i{{64|32}})* [[OMP_OUTLINED:@.+]] to void (i32*, i32*, ...)*), i8*** [[ARGC_ADDR]], double** [[VAR:%.+]], i{{64|32}} %{{.+}})
// ALL: ret i32 0
// ALL-NEXT: }
// ALL-DEBUG: define linkonce_odr i32 [[TMAIN]](i8** %argc)
@@ -124,12 +124,12 @@
// CHECK-DEBUG: [[KMPC_LOC_PSOURCE_REF:%.+]] = getelementptr inbounds %struct.ident_t, %struct.ident_t* [[LOC_2_ADDR]], i32 0, i32 4
// CHECK-DEBUG-NEXT: store i8* getelementptr inbounds ([{{.+}} x i8], [{{.+}} x i8]* [[LOC2]], i32 0, i32 0), i8** [[KMPC_LOC_PSOURCE_REF]]
// CHECK-DEBUG-NEXT: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* [[LOC_2_ADDR]], i32 2, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i8***, i64)* [[OMP_OUTLINED:@.+]] to void (i32*, i32*, ...)*), i8*** [[ARGC_ADDR]], i64 %{{.+}})
-// IRBUILDER-DEBUG: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @{{.*}}, i32 2, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i8***, i64)* [[OMP_OUTLINED:@.+]] to void (i32*, i32*, ...)*), i8*** [[ARGC_ADDR]], i64 %{{.+}})
+// IRBUILDER-DEBUG: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @{{.*}}, i32 3, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i8***, double**, i64)* [[OMP_OUTLINED:@.+]] to void (i32*, i32*, ...)*), i8*** [[ARGC_ADDR]], double** [[VAR:%.+]], i64 %{{.+}})
// ALL-DEBUG: ret i32 0
// ALL-DEBUG-NEXT: }
// CHECK: define internal {{.*}}void [[OMP_OUTLINED]](i32* noalias %.global_tid., i32* noalias %.bound_tid., i8*** dereferenceable({{4|8}}) %argc, i{{64|32}}{{.*}} %{{.+}})
-// IRBUILDER: define internal {{.*}}void [[OMP_OUTLINED]](i32* noalias %{{.*}}, i32* noalias %{{.*}}, i8*** [[ARGC_REF:%.*]], i{{64|32}}{{.*}} %{{.+}})
+// IRBUILDER: define internal {{.*}}void [[OMP_OUTLINED]](i32* noalias %{{.*}}, i32* noalias %{{.*}}, i8*** [[ARGC_REF:%.*]], double** [[VAR]], i{{64|32}}{{.*}} %{{.+}})
// CHECK: store i8*** %argc, i8 [[ARGC_PTR_ADDR:%.+]],
// CHECK: [[ARGC_REF:%.+]] = load i8***, i8 [[ARGC_PTR_ADDR]]
// ALL: [[ARGC:%.+]] = load i8**, i8*** [[ARGC_REF]]
@@ -140,7 +140,7 @@
// CHECK-NEXT: unreachable
// CHECK-NEXT: }
// CHECK-DEBUG: define internal void [[OMP_OUTLINED_DEBUG:@.+]](i32* noalias %.global_tid., i32* noalias %.bound_tid., i8*** dereferenceable({{4|8}}) %argc, i64 %{{.+}})
-// IRBUILDER-DEBUG: define internal void [[OMP_OUTLINED_DEBUG:@.+]](i32* noalias %{{.*}}, i32* noalias %{{.*}}, i8*** [[ARGC_REF:%.*]], i64 %{{.+}})
+// IRBUILDER-DEBUG: define internal void [[OMP_OUTLINED_DEBUG:@.+]](i32* noalias %{{.*}}, i32* noalias %{{.*}}, i8*** [[ARGC_REF:%.*]], double** [[VAR]], i64 %{{.+}})
// CHECK-DEBUG: store i8*** %argc, i8 [[ARGC_PTR_
