Skip to content

Commit

Permalink
[OpenMPIRBuilder] Remove ContinuationBB argument from Body callback.
Browse files Browse the repository at this point in the history 
The callback is expected to create a branch to the ContinuationBB (sometimes called FiniBB in some lambdas) argument when finishing. This creates problems:

 1. The InsertPoint used for CodeGenIP does not need to be the end of a block. If it is not, a naive callback will insert a branch instruction into the middle of the block.

 2. The BasicBlock the CodeGenIP is pointing to may or may not have a terminator. There is an conflict where to branch to if the block already has a terminator.

 3. Some API functions work only with block having a terminator. Some workarounds have been used to insert a temporary terminator that is removed again.

 4. Some callbacks are sensitive to whether the BasicBlock has a terminator or not. This creates a callback ordering problem where different callback may have different behaviour depending on whether a previous callback created a terminator or not. The problem also exists for FinalizeCallbackTy where some callbacks do create branch to another "continue" block, but unlike BodyGenCallbackTy does not receive the target as argument. This is not addressed in this patch.

With this patch, the callback receives an CodeGenIP into a BasicBlock where to insert instructions. If it has to insert control flow, it can split the block at that position as needed but otherwise no separate ContinuationBB is needed. In particular, a callback can be empty without breaking the emitted IR. If the caller needs the control flow to branch to a specific target, it can insert the branch instruction itself and pass an InsertPoint before the terminator to the callback.

Certain frontends such as Clang may expect the current IRBuilder position to be at the end of a basic block. In this case its callbacks must split the block at CodeGenIP before setting the IRBuilder position such that the instructions after CodeGenIP are moved to another basic block and before returning create a new branch instruction to the split block.

Some utility functions such as `splitBB` are supporting correct splitting of BasicBlocks, independent of whether they have a terminator or not, returning/setting the InsertPoint of an IRBuilder to the end of split predecessor block, and optionally omitting creating a branch to the split successor block to be added later.

Reviewed By: kiranchandramohan

Differential Revision: https://reviews.llvm.org/D118409
  • Loading branch information
@Meinersbur
Meinersbur committed Apr 26, 2022
1 parent 077488a commit ff289fe
Show file tree
Hide file tree
Showing 14 changed files with 3,295 additions and 879 deletions.
110 changes: 65 additions & 45 deletions clang/lib/CodeGen/CGStmtOpenMP.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1677,6 +1677,41 @@ std::string CodeGenFunction::OMPBuilderCBHelpers::getNameWithSeparators(
}
return OS.str().str();
}

void CodeGenFunction::OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
CodeGenFunction &CGF, const Stmt *RegionBodyStmt, InsertPointTy AllocaIP,
InsertPointTy CodeGenIP, Twine RegionName) {
CGBuilderTy &Builder = CGF.Builder;
Builder.restoreIP(CodeGenIP);
llvm::BasicBlock *FiniBB = splitBBWithSuffix(Builder, /*CreateBranch=*/false,
"." + RegionName + ".after");

{
OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(CGF, AllocaIP, *FiniBB);
CGF.EmitStmt(RegionBodyStmt);
}

if (Builder.saveIP().isSet())
Builder.CreateBr(FiniBB);
}

void CodeGenFunction::OMPBuilderCBHelpers::EmitOMPOutlinedRegionBody(
CodeGenFunction &CGF, const Stmt *RegionBodyStmt, InsertPointTy AllocaIP,
InsertPointTy CodeGenIP, Twine RegionName) {
CGBuilderTy &Builder = CGF.Builder;
Builder.restoreIP(CodeGenIP);
llvm::BasicBlock *FiniBB = splitBBWithSuffix(Builder, /*CreateBranch=*/false,
"." + RegionName + ".after");

{
OMPBuilderCBHelpers::OutlinedRegionBodyRAII IRB(CGF, AllocaIP, *FiniBB);
CGF.EmitStmt(RegionBodyStmt);
}

if (Builder.saveIP().isSet())
Builder.CreateBr(FiniBB);
}

void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
if (CGM.getLangOpts().OpenMPIRBuilder) {
llvm::OpenMPIRBuilder &OMPBuilder = CGM.getOpenMPRuntime().getOMPBuilder();
Expand Down Expand Up @@ -1719,13 +1754,10 @@ void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
const CapturedStmt *CS = S.getCapturedStmt(OMPD_parallel);
const Stmt *ParallelRegionBodyStmt = CS->getCapturedStmt();

auto BodyGenCB = [ParallelRegionBodyStmt,
this](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
llvm::BasicBlock &ContinuationBB) {
OMPBuilderCBHelpers::OutlinedRegionBodyRAII ORB(*this, AllocaIP,
ContinuationBB);
OMPBuilderCBHelpers::EmitOMPRegionBody(*this, ParallelRegionBodyStmt,
CodeGenIP, ContinuationBB);
auto BodyGenCB = [&, this](InsertPointTy AllocaIP,
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPOutlinedRegionBody(
*this, ParallelRegionBodyStmt, AllocaIP, CodeGenIP, "parallel");
};

CGCapturedStmtInfo CGSI(*CS, CR_OpenMP);
Expand Down Expand Up @@ -3983,22 +4015,17 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
if (CS) {
for (const Stmt *SubStmt : CS->children()) {
auto SectionCB = [this, SubStmt](InsertPointTy AllocaIP,
InsertPointTy CodeGenIP,
llvm::BasicBlock &FiniBB) {
OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP,
FiniBB);
OMPBuilderCBHelpers::EmitOMPRegionBody(*this, SubStmt, CodeGenIP,
FiniBB);
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, SubStmt, AllocaIP, CodeGenIP, "section");
};
SectionCBVector.push_back(SectionCB);
}
} else {
auto SectionCB = [this, CapturedStmt](InsertPointTy AllocaIP,
InsertPointTy CodeGenIP,
llvm::BasicBlock &FiniBB) {
OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP, FiniBB);
OMPBuilderCBHelpers::EmitOMPRegionBody(*this, CapturedStmt, CodeGenIP,
FiniBB);
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, CapturedStmt, AllocaIP, CodeGenIP, "section");
};
SectionCBVector.push_back(SectionCB);
}
Expand Down Expand Up @@ -4051,11 +4078,9 @@ void CodeGenFunction::EmitOMPSectionDirective(const OMPSectionDirective &S) {
};

auto BodyGenCB = [SectionRegionBodyStmt, this](InsertPointTy AllocaIP,
InsertPointTy CodeGenIP,
llvm::BasicBlock &FiniBB) {
OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP, FiniBB);
OMPBuilderCBHelpers::EmitOMPRegionBody(*this, SectionRegionBodyStmt,
CodeGenIP, FiniBB);
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, SectionRegionBodyStmt, AllocaIP, CodeGenIP, "section");
};

LexicalScope Scope(*this, S.getSourceRange());
Expand Down Expand Up @@ -4134,11 +4159,9 @@ void CodeGenFunction::EmitOMPMasterDirective(const OMPMasterDirective &S) {
};

auto BodyGenCB = [MasterRegionBodyStmt, this](InsertPointTy AllocaIP,
InsertPointTy CodeGenIP,
llvm::BasicBlock &FiniBB) {
OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP, FiniBB);
OMPBuilderCBHelpers::EmitOMPRegionBody(*this, MasterRegionBodyStmt,
CodeGenIP, FiniBB);
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, MasterRegionBodyStmt, AllocaIP, CodeGenIP, "master");
};

LexicalScope Scope(*this, S.getSourceRange());
Expand Down Expand Up @@ -4182,11 +4205,9 @@ void CodeGenFunction::EmitOMPMaskedDirective(const OMPMaskedDirective &S) {
};

auto BodyGenCB = [MaskedRegionBodyStmt, this](InsertPointTy AllocaIP,
InsertPointTy CodeGenIP,
llvm::BasicBlock &FiniBB) {
OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP, FiniBB);
OMPBuilderCBHelpers::EmitOMPRegionBody(*this, MaskedRegionBodyStmt,
CodeGenIP, FiniBB);
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, MaskedRegionBodyStmt, AllocaIP, CodeGenIP, "masked");
};

LexicalScope Scope(*this, S.getSourceRange());
Expand Down Expand Up @@ -4224,11 +4245,9 @@ void CodeGenFunction::EmitOMPCriticalDirective(const OMPCriticalDirective &S) {
};

auto BodyGenCB = [CriticalRegionBodyStmt, this](InsertPointTy AllocaIP,
InsertPointTy CodeGenIP,
llvm::BasicBlock &FiniBB) {
OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP, FiniBB);
OMPBuilderCBHelpers::EmitOMPRegionBody(*this, CriticalRegionBodyStmt,
CodeGenIP, FiniBB);
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, CriticalRegionBodyStmt, AllocaIP, CodeGenIP, "critical");
};

LexicalScope Scope(*this, S.getSourceRange());
Expand Down Expand Up @@ -5564,24 +5583,25 @@ void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) {
};

auto BodyGenCB = [&S, C, this](InsertPointTy AllocaIP,
InsertPointTy CodeGenIP,
llvm::BasicBlock &FiniBB) {
InsertPointTy CodeGenIP) {
Builder.restoreIP(CodeGenIP);

const CapturedStmt *CS = S.getInnermostCapturedStmt();
if (C) {
llvm::BasicBlock *FiniBB = splitBBWithSuffix(
Builder, /*CreateBranch=*/false, ".ordered.after");
llvm::SmallVector<llvm::Value *, 16> CapturedVars;
GenerateOpenMPCapturedVars(*CS, CapturedVars);
llvm::Function *OutlinedFn =
emitOutlinedOrderedFunction(CGM, CS, S.getBeginLoc());
assert(S.getBeginLoc().isValid() &&
"Outlined function call location must be valid.");
ApplyDebugLocation::CreateDefaultArtificial(*this, S.getBeginLoc());
OMPBuilderCBHelpers::EmitCaptureStmt(*this, CodeGenIP, FiniBB,
OMPBuilderCBHelpers::EmitCaptureStmt(*this, CodeGenIP, *FiniBB,
OutlinedFn, CapturedVars);
} else {
OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP,
FiniBB);
OMPBuilderCBHelpers::EmitOMPRegionBody(*this, CS->getCapturedStmt(),
CodeGenIP, FiniBB);
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, CS->getCapturedStmt(), AllocaIP, CodeGenIP, "ordered");
}
};

Expand Down
48 changes: 25 additions & 23 deletions clang/lib/CodeGen/CodeGenFunction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1791,26 +1791,17 @@ class CodeGenFunction : public CodeGenTypeCache {
}

/// 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);
}
/// \param CGF The Codegen function this belongs to
/// \param RegionBodyStmt The body statement for the OpenMP region being
/// generated
/// \param AllocaIP Where to insert alloca instructions
/// \param CodeGenIP Where to insert the region code
/// \param RegionName Name to be used for new blocks
static void EmitOMPInlinedRegionBody(CodeGenFunction &CGF,
const Stmt *RegionBodyStmt,
InsertPointTy AllocaIP,
InsertPointTy CodeGenIP,
Twine RegionName);

static void EmitCaptureStmt(CodeGenFunction &CGF, InsertPointTy CodeGenIP,
llvm::BasicBlock &FiniBB, llvm::Function *Fn,
Expand All @@ -1830,12 +1821,25 @@ class CodeGenFunction : public CodeGenTypeCache {
CGF.Builder.CreateBr(&FiniBB);
}

/// Emit the body of an OMP region that will be outlined in
/// OpenMPIRBuilder::finalize().
/// \param CGF The Codegen function this belongs to
/// \param RegionBodyStmt The body statement for the OpenMP region being
/// generated
/// \param AllocaIP Where to insert alloca instructions
/// \param CodeGenIP Where to insert the region code
/// \param RegionName Name to be used for new blocks
static void EmitOMPOutlinedRegionBody(CodeGenFunction &CGF,
const Stmt *RegionBodyStmt,
InsertPointTy AllocaIP,
InsertPointTy CodeGenIP,
Twine RegionName);

/// RAII for preserving necessary info during Outlined region body codegen.
class OutlinedRegionBodyRAII {

llvm::AssertingVH<llvm::Instruction> OldAllocaIP;
CodeGenFunction::JumpDest OldReturnBlock;
CGBuilderTy::InsertPoint IP;
CodeGenFunction &CGF;

public:
Expand All @@ -1846,7 +1850,6 @@ class CodeGenFunction : public CodeGenTypeCache {
"Must specify Insertion point for allocas of outlined function");
OldAllocaIP = CGF.AllocaInsertPt;
CGF.AllocaInsertPt = &*AllocaIP.getPoint();
IP = CGF.Builder.saveIP();

OldReturnBlock = CGF.ReturnBlock;
CGF.ReturnBlock = CGF.getJumpDestInCurrentScope(&RetBB);
Expand All @@ -1855,7 +1858,6 @@ class CodeGenFunction : public CodeGenTypeCache {
~OutlinedRegionBodyRAII() {
CGF.AllocaInsertPt = OldAllocaIP;
CGF.ReturnBlock = OldReturnBlock;
CGF.Builder.restoreIP(IP);
}
};

Expand Down
Loading

0 comments on commit ff289fe

Please sign in to comment.