https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/153610
>From 516b27dcd806917054f56a8545b5f00b855f6680 Mon Sep 17 00:00:00 2001 From: erichkeane <eke...@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 <eke...@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 <eke...@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 = _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits