[clang] [CIR] Refactor recipe init generation, cleanup after init (PR #153610)
Erich Keane via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 14 13:03:11 PDT 2025
https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/153610
>From 516b27dcd806917054f56a8545b5f00b855f6680 Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Thu, 14 Aug 2025 09:29:59 -0700
Subject: [PATCH 1/3] [CIR] Refactor recipe init generation, cleanup after init
In preperation of the firstprivate implementation, this separates out
some functions to make it easier to read.
Additionally, it cleans up the VarDecl->alloca relationship, which
will prevent issues if we have to re-use the same vardecl for a future
generated recipe (and causes concerns in firstprivate later).
---
clang/lib/CIR/CodeGen/CIRGenFunction.h | 7 +
clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp | 179 ++++++++++++------
2 files changed, 123 insertions(+), 63 deletions(-)
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index ddc1edd77010c..6a22a217c62e6 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -502,6 +502,13 @@ class CIRGenFunction : public CIRGenTypeCache {
// TODO: Add symbol table support
}
+ /// Removes a declaration from the address-relationship. This is a function
+ /// that shouldn't need to be used except in cases where we're adding/removing
+ /// things that aren't part of the language-semantics AST.
+ void removeAddrOfLocalVar(const clang::VarDecl *vd) {
+ localDeclMap.erase(vd);
+ }
+
bool shouldNullCheckClassCastValue(const CastExpr *ce);
RValue convertTempToRValue(Address addr, clang::QualType type,
diff --git a/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp b/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp
index bb9054a68b5c7..978a46ff8fe6c 100644
--- a/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp
@@ -357,15 +357,11 @@ class OpenACCClauseCIREmitter final
}
template <typename RecipeTy>
- RecipeTy getOrCreateRecipe(ASTContext &astCtx, const Expr *varRef,
- const VarDecl *varRecipe, DeclContext *dc,
- QualType baseType, mlir::Value mainOp) {
- mlir::ModuleOp mod =
- builder.getBlock()->getParent()->getParentOfType<mlir::ModuleOp>();
-
+ std::string getRecipeName(SourceRange loc, QualType baseType) {
std::string recipeName;
{
llvm::raw_string_ostream stream(recipeName);
+
if constexpr (std::is_same_v<RecipeTy, mlir::acc::PrivateRecipeOp>) {
stream << "privatization_";
} else if constexpr (std::is_same_v<RecipeTy,
@@ -378,8 +374,7 @@ class OpenACCClauseCIREmitter final
// We don't have the reduction operation here well enough to know how to
// spell this correctly (+ == 'add', etc), so when we implement
// 'reduction' we have to do that here.
- cgf.cgm.errorNYI(varRef->getSourceRange(),
- "OpeNACC reduction recipe creation");
+ cgf.cgm.errorNYI(loc, "OpenACC reduction recipe name creation");
} else {
static_assert(!sizeof(RecipeTy), "Unknown Recipe op kind");
}
@@ -387,7 +382,113 @@ class OpenACCClauseCIREmitter final
MangleContext &mc = cgf.cgm.getCXXABI().getMangleContext();
mc.mangleCanonicalTypeName(baseType, stream);
}
+ return recipeName;
+ }
+
+ // Create the 'init' section of the recipe, including the 'copy' section for
+ // 'firstprivate'.
+ template <typename RecipeTy>
+ void createRecipeInitCopy(mlir::Location loc, mlir::Location locEnd,
+ SourceRange exprRange, mlir::Value mainOp,
+ RecipeTy recipe, const VarDecl *varRecipe,
+ const VarDecl *temporary) {
+ assert(varRecipe && "Required recipe variable not set?");
+ if constexpr (std::is_same_v<RecipeTy, mlir::acc::ReductionRecipeOp>) {
+ // We haven't implemented the 'init' recipe for Reduction yet, so NYI
+ // it.
+ cgf.cgm.errorNYI(exprRange, "OpenACC Reduction recipe init");
+ }
+
+ if constexpr (std::is_same_v<RecipeTy, mlir::acc::FirstprivateRecipeOp>) {
+ // We haven't implemented the 'init'/copy recipe for firstprivate yet, so
+ // NYI it.
+ cgf.cgm.errorNYI(exprRange, "OpenACC firstprivate recipe init");
+ }
+
+ CIRGenFunction::AutoVarEmission tempDeclEmission{
+ CIRGenFunction::AutoVarEmission::invalid()};
+
+ // Do the 'init' section of the recipe IR, which does an alloca, then the
+ // initialization (except for firstprivate).
+ llvm::SmallVector<mlir::Type> argsTys{mainOp.getType()};
+ llvm::SmallVector<mlir::Location> argsLocs{loc};
+ builder.createBlock(&recipe.getInitRegion(), recipe.getInitRegion().end(),
+ argsTys, argsLocs);
+ builder.setInsertionPointToEnd(&recipe.getInitRegion().back());
+ tempDeclEmission =
+ cgf.emitAutoVarAlloca(*varRecipe, builder.saveInsertionPoint());
+ // 'firstprivate' doesn't do its initialization in the 'init' section,
+ // instead does it in the 'copy' section. SO only do init here.
+ // 'reduction' appears to use it too (rather than a 'copy' section), so
+ // we probably have to do it here too, but we can do that when we get to
+ // reduction implementation.
+ if constexpr (std::is_same_v<RecipeTy, mlir::acc::PrivateRecipeOp>) {
+ // We are OK with no init for builtins, arrays of builtins, or pointers,
+ // else we should NYI so we know to go look for these.
+ if (!varRecipe->getType()
+ ->getPointeeOrArrayElementType()
+ ->isBuiltinType() &&
+ !varRecipe->getType()->isPointerType() && !varRecipe->getInit()) {
+ // If we don't have any initialization recipe, we failed during Sema to
+ // initialize this correctly. If we disable the
+ // Sema::TentativeAnalysisScopes in SemaOpenACC::CreateInitRecipe, it'll
+ // emit an error to tell us. However, emitting those errors during
+ // production is a violation of the standard, so we cannot do them.
+ cgf.cgm.errorNYI(exprRange, "private default-init recipe");
+ }
+ cgf.emitAutoVarInit(tempDeclEmission);
+ }
+
+ mlir::acc::YieldOp::create(builder, locEnd);
+
+ if constexpr (std::is_same_v<RecipeTy, mlir::acc::FirstprivateRecipeOp>) {
+ if (!varRecipe->getInit()) {
+ // If we don't have any initialization recipe, we failed during Sema to
+ // initialize this correctly. If we disable the
+ // Sema::TentativeAnalysisScopes in SemaOpenACC::CreateInitRecipe, it'll
+ // emit an error to tell us. However, emitting those errors during
+ // production is a violation of the standard, so we cannot do them.
+ cgf.cgm.errorNYI(
+ exprRange, "firstprivate copy-init recipe not properly generated");
+ }
+
+ cgf.cgm.errorNYI(exprRange, "firstprivate copy section generation");
+ }
+
+ // Make sure we cleanup after ourselves here.
+ cgf.removeAddrOfLocalVar(varRecipe);
+ }
+
+ void createRecipeDestroySection(mlir::Location loc, mlir::Location locEnd,
+ mlir::Value mainOp, CharUnits alignment,
+ QualType baseType,
+ mlir::Region &destroyRegion) {
+ llvm::SmallVector<mlir::Type> argsTys{mainOp.getType()};
+ llvm::SmallVector<mlir::Location> argsLocs{loc};
+ mlir::Block *block = builder.createBlock(
+ &destroyRegion, destroyRegion.end(), argsTys, argsLocs);
+ builder.setInsertionPointToEnd(&destroyRegion.back());
+
+ mlir::Type elementTy =
+ mlir::cast<cir::PointerType>(mainOp.getType()).getPointee();
+ Address addr{block->getArgument(0), elementTy, alignment};
+ cgf.emitDestroy(addr, baseType,
+ cgf.getDestroyer(QualType::DK_cxx_destructor));
+
+ mlir::acc::YieldOp::create(builder, locEnd);
+ }
+
+ template <typename RecipeTy>
+ RecipeTy getOrCreateRecipe(ASTContext &astCtx, const Expr *varRef,
+ const VarDecl *varRecipe, const VarDecl *temporary,
+ DeclContext *dc, QualType baseType,
+ mlir::Value mainOp) {
+ mlir::ModuleOp mod =
+ builder.getBlock()->getParent()->getParentOfType<mlir::ModuleOp>();
+
+ std::string recipeName =
+ getRecipeName<RecipeTy>(varRef->getSourceRange(), baseType);
if (auto recipe = mod.lookupSymbol<RecipeTy>(recipeName))
return recipe;
@@ -398,61 +499,13 @@ class OpenACCClauseCIREmitter final
auto recipe =
RecipeTy::create(modBuilder, loc, recipeName, mainOp.getType());
- CIRGenFunction::AutoVarEmission tempDeclEmission{
- CIRGenFunction::AutoVarEmission::invalid()};
-
- // Init section.
- {
- llvm::SmallVector<mlir::Type> argsTys{mainOp.getType()};
- llvm::SmallVector<mlir::Location> argsLocs{loc};
- builder.createBlock(&recipe.getInitRegion(), recipe.getInitRegion().end(),
- argsTys, argsLocs);
- builder.setInsertionPointToEnd(&recipe.getInitRegion().back());
-
- if constexpr (!std::is_same_v<RecipeTy, mlir::acc::PrivateRecipeOp>) {
- // We have only implemented 'init' for private, so make this NYI until
- // we have explicitly implemented everything.
- cgf.cgm.errorNYI(varRef->getSourceRange(),
- "OpenACC non-private recipe init");
- }
-
- if (varRecipe) {
- tempDeclEmission =
- cgf.emitAutoVarAlloca(*varRecipe, builder.saveInsertionPoint());
- cgf.emitAutoVarInit(tempDeclEmission);
- }
-
- mlir::acc::YieldOp::create(builder, locEnd);
- }
-
- // Copy section.
- if constexpr (std::is_same_v<RecipeTy, mlir::acc::FirstprivateRecipeOp> ||
- std::is_same_v<RecipeTy, mlir::acc::ReductionRecipeOp>) {
- // TODO: OpenACC: 'private' doesn't emit this, but for the other two we
- // have to figure out what 'copy' means here.
- cgf.cgm.errorNYI(varRef->getSourceRange(),
- "OpenACC record type privatization copy section");
- }
-
- // Destroy section (doesn't currently exist).
- if (varRecipe && varRecipe->needsDestruction(cgf.getContext())) {
- llvm::SmallVector<mlir::Type> argsTys{mainOp.getType()};
- llvm::SmallVector<mlir::Location> argsLocs{loc};
- mlir::Block *block = builder.createBlock(&recipe.getDestroyRegion(),
- recipe.getDestroyRegion().end(),
- argsTys, argsLocs);
- builder.setInsertionPointToEnd(&recipe.getDestroyRegion().back());
-
- mlir::Type elementTy =
- mlir::cast<cir::PointerType>(mainOp.getType()).getPointee();
- Address addr{block->getArgument(0), elementTy,
- cgf.getContext().getDeclAlign(varRecipe)};
- cgf.emitDestroy(addr, baseType,
- cgf.getDestroyer(QualType::DK_cxx_destructor));
-
- mlir::acc::YieldOp::create(builder, locEnd);
- }
+ createRecipeInitCopy(loc, locEnd, varRef->getSourceRange(), mainOp, recipe,
+ varRecipe, temporary);
+ if (varRecipe && varRecipe->needsDestruction(cgf.getContext()))
+ createRecipeDestroySection(loc, locEnd, mainOp,
+ cgf.getContext().getDeclAlign(varRecipe),
+ baseType, recipe.getDestroyRegion());
return recipe;
}
@@ -1088,7 +1141,7 @@ class OpenACCClauseCIREmitter final
{
mlir::OpBuilder::InsertionGuard guardCase(builder);
auto recipe = getOrCreateRecipe<mlir::acc::PrivateRecipeOp>(
- cgf.getContext(), varExpr, varRecipe,
+ cgf.getContext(), varExpr, varRecipe, /*temporary=*/nullptr,
Decl::castToDeclContext(cgf.curFuncDecl), opInfo.baseType,
privateOp.getResult());
// TODO: OpenACC: The dialect is going to change in the near future to
>From a14719ae97416885d3a2855a46ff4025fda0a57c Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Thu, 14 Aug 2025 09:44:46 -0700
Subject: [PATCH 2/3] Clang-format
---
clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp b/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp
index 978a46ff8fe6c..8ab52152f06c8 100644
--- a/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp
@@ -405,8 +405,8 @@ class OpenACCClauseCIREmitter final
cgf.cgm.errorNYI(exprRange, "OpenACC firstprivate recipe init");
}
- CIRGenFunction::AutoVarEmission tempDeclEmission{
- CIRGenFunction::AutoVarEmission::invalid()};
+ CIRGenFunction::AutoVarEmission tempDeclEmission{
+ CIRGenFunction::AutoVarEmission::invalid()};
// Do the 'init' section of the recipe IR, which does an alloca, then the
// initialization (except for firstprivate).
>From d68c9074e4dfec3dc2bbbf2a5c0e8a05a396c772 Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Thu, 14 Aug 2025 13:02:58 -0700
Subject: [PATCH 3/3] Make changes suggested by Andy
---
clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp b/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp
index 8ab52152f06c8..9194b522114bc 100644
--- a/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp
@@ -371,9 +371,11 @@ class OpenACCClauseCIREmitter final
} else if constexpr (std::is_same_v<RecipeTy,
mlir::acc::ReductionRecipeOp>) {
stream << "reduction_";
- // We don't have the reduction operation here well enough to know how to
- // spell this correctly (+ == 'add', etc), so when we implement
- // 'reduction' we have to do that here.
+ // TODO: OpenACC: once we have this part implemented, we can remove the
+ // SourceRange `loc` variable from this function. We don't have the
+ // reduction operation here well enough to know how to spell this
+ // correctly (+ == 'add', etc), so when we implement 'reduction' we have
+ // to do that here.
cgf.cgm.errorNYI(loc, "OpenACC reduction recipe name creation");
} else {
static_assert(!sizeof(RecipeTy), "Unknown Recipe op kind");
@@ -410,10 +412,8 @@ class OpenACCClauseCIREmitter final
// Do the 'init' section of the recipe IR, which does an alloca, then the
// initialization (except for firstprivate).
- llvm::SmallVector<mlir::Type> argsTys{mainOp.getType()};
- llvm::SmallVector<mlir::Location> argsLocs{loc};
builder.createBlock(&recipe.getInitRegion(), recipe.getInitRegion().end(),
- argsTys, argsLocs);
+ {mainOp.getType()}, {loc});
builder.setInsertionPointToEnd(&recipe.getInitRegion().back());
tempDeclEmission =
cgf.emitAutoVarAlloca(*varRecipe, builder.saveInsertionPoint());
@@ -463,11 +463,8 @@ class OpenACCClauseCIREmitter final
mlir::Value mainOp, CharUnits alignment,
QualType baseType,
mlir::Region &destroyRegion) {
- llvm::SmallVector<mlir::Type> argsTys{mainOp.getType()};
- llvm::SmallVector<mlir::Location> argsLocs{loc};
-
mlir::Block *block = builder.createBlock(
- &destroyRegion, destroyRegion.end(), argsTys, argsLocs);
+ &destroyRegion, destroyRegion.end(), {mainOp.getType()}, {loc});
builder.setInsertionPointToEnd(&destroyRegion.back());
mlir::Type elementTy =
More information about the cfe-commits
mailing list