Skip to content

Commit 41387ab

Browse files
authored
[CIR][OpenACC] Implement pointer/array recipe destructors (#160189)
After previous implementation, I discovered that we were both doing arrays incorrectly for recipes, plus didn't get the pointer allocations done correctly. This patch is the first of a few in a series that attempts to make sure we get all pointers/arrays correct. This patch is limited to just 'private' and destructors, which simplifies the review significantly. Destructors are simply looped through and called at each level. The 'recipe-decl' is the 'least bounded' (that is, the type of the expression, in the type of `int[5] i; #pragma acc parallel private(i[1])`, the type of the `recipe-decl` is `int`. This allows us to do init/destruction at the element level. This patch also adds infrastructure for the rest of the series of private (for the init section), as well as extensive testing for 'private', with a lot of 'TODO' locations. Future patches will fill these in, but at the moment, there is an NYI warning for bounds, so a number of tests are updated to handle that.
1 parent 8f7cfd4 commit 41387ab

File tree

41 files changed

+3313
-343
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+3313
-343
lines changed

clang/include/clang/AST/OpenACCClause.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -842,9 +842,7 @@ struct OpenACCPrivateRecipe {
842842
VarDecl *AllocaDecl;
843843
Expr *InitExpr;
844844

845-
OpenACCPrivateRecipe(VarDecl *A, Expr *I) : AllocaDecl(A), InitExpr(I) {
846-
assert(!AllocaDecl || AllocaDecl->getInit() == nullptr);
847-
}
845+
OpenACCPrivateRecipe(VarDecl *A, Expr *I) : AllocaDecl(A), InitExpr(I) {}
848846

849847
bool isSet() const { return AllocaDecl; }
850848

clang/lib/CIR/CodeGen/CIRGenFunction.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1723,8 +1723,15 @@ class CIRGenFunction : public CIRGenTypeCache {
17231723
mlir::Location beginLoc;
17241724
mlir::Value varValue;
17251725
std::string name;
1726+
// The type of the original variable reference: that is, after 'bounds' have
1727+
// removed pointers/array types/etc. So in the case of int arr[5], and a
1728+
// private(arr[1]), 'origType' is 'int', but 'baseType' is 'int[5]'.
1729+
QualType origType;
17261730
QualType baseType;
17271731
llvm::SmallVector<mlir::Value> bounds;
1732+
// The list of types that we found when going through the bounds, which we
1733+
// can use to properly set the alloca section.
1734+
llvm::SmallVector<QualType> boundTypes;
17281735
};
17291736
// Gets the collection of info required to lower and OpenACC clause or cache
17301737
// construct variable reference.

clang/lib/CIR/CodeGen/CIRGenOpenACC.cpp

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,33 @@ mlir::Value CIRGenFunction::createOpenACCConstantInt(mlir::Location loc,
6868
CIRGenFunction::OpenACCDataOperandInfo
6969
CIRGenFunction::getOpenACCDataOperandInfo(const Expr *e) {
7070
const Expr *curVarExpr = e->IgnoreParenImpCasts();
71+
QualType origType =
72+
curVarExpr->getType().getNonReferenceType().getUnqualifiedType();
73+
// Array sections are special, and we have to treat them that way.
74+
if (const auto *section =
75+
dyn_cast<ArraySectionExpr>(curVarExpr->IgnoreParenImpCasts()))
76+
origType = ArraySectionExpr::getBaseOriginalType(section);
7177

7278
mlir::Location exprLoc = cgm.getLoc(curVarExpr->getBeginLoc());
7379
llvm::SmallVector<mlir::Value> bounds;
80+
llvm::SmallVector<QualType> boundTypes;
7481

7582
std::string exprString;
7683
llvm::raw_string_ostream os(exprString);
7784
e->printPretty(os, nullptr, getContext().getPrintingPolicy());
7885

86+
auto addBoundType = [&](const Expr *e) {
87+
if (const auto *section = dyn_cast<ArraySectionExpr>(curVarExpr)) {
88+
QualType baseTy = ArraySectionExpr::getBaseOriginalType(
89+
section->getBase()->IgnoreParenImpCasts());
90+
boundTypes.push_back(QualType(baseTy->getPointeeOrArrayElementType(), 0));
91+
} else {
92+
boundTypes.push_back(curVarExpr->getType());
93+
}
94+
};
95+
96+
addBoundType(curVarExpr);
97+
7998
while (isa<ArraySectionExpr, ArraySubscriptExpr>(curVarExpr)) {
8099
mlir::Location boundLoc = cgm.getLoc(curVarExpr->getBeginLoc());
81100
mlir::Value lowerBound;
@@ -115,19 +134,28 @@ CIRGenFunction::getOpenACCDataOperandInfo(const Expr *e) {
115134

116135
bounds.push_back(createBound(*this, this->builder, boundLoc, lowerBound,
117136
upperBound, extent));
137+
addBoundType(curVarExpr);
118138
}
119139

120140
if (const auto *memExpr = dyn_cast<MemberExpr>(curVarExpr))
121-
return {exprLoc, emitMemberExpr(memExpr).getPointer(), exprString,
141+
return {exprLoc,
142+
emitMemberExpr(memExpr).getPointer(),
143+
exprString,
144+
origType,
122145
curVarExpr->getType().getNonReferenceType().getUnqualifiedType(),
123-
std::move(bounds)};
146+
std::move(bounds),
147+
std::move(boundTypes)};
124148

125149
// Sema has made sure that only 4 types of things can get here, array
126150
// subscript, array section, member expr, or DRE to a var decl (or the
127151
// former 3 wrapping a var-decl), so we should be able to assume this is
128152
// right.
129153
const auto *dre = cast<DeclRefExpr>(curVarExpr);
130-
return {exprLoc, emitDeclRefLValue(dre).getPointer(), exprString,
154+
return {exprLoc,
155+
emitDeclRefLValue(dre).getPointer(),
156+
exprString,
157+
origType,
131158
curVarExpr->getType().getNonReferenceType().getUnqualifiedType(),
132-
std::move(bounds)};
159+
std::move(bounds),
160+
std::move(boundTypes)};
133161
}

clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -988,20 +988,16 @@ class OpenACCClauseCIREmitter final
988988

989989
{
990990
mlir::OpBuilder::InsertionGuard guardCase(builder);
991-
// TODO: OpenACC: At the moment this is a bit of a hacky way of doing
992-
// this, and won't work when we get to bounds/etc. Do this for now to
993-
// limit the scope of this refactor.
994-
VarDecl *allocaDecl = varRecipe.AllocaDecl;
995-
allocaDecl->setInit(varRecipe.InitExpr);
996-
allocaDecl->setInitStyle(VarDecl::CallInit);
997991

998992
auto recipe =
999993
OpenACCRecipeBuilder<mlir::acc::PrivateRecipeOp>(cgf, builder)
1000-
.getOrCreateRecipe(cgf.getContext(), varExpr, allocaDecl,
1001-
/*temporary=*/nullptr,
1002-
OpenACCReductionOperator::Invalid,
1003-
Decl::castToDeclContext(cgf.curFuncDecl),
1004-
opInfo.baseType, privateOp.getResult());
994+
.getOrCreateRecipe(
995+
cgf.getContext(), varExpr, varRecipe.AllocaDecl,
996+
varRecipe.InitExpr,
997+
/*temporary=*/nullptr, OpenACCReductionOperator::Invalid,
998+
Decl::castToDeclContext(cgf.curFuncDecl), opInfo.origType,
999+
opInfo.bounds.size(), opInfo.boundTypes, opInfo.baseType,
1000+
privateOp.getResult());
10051001
// TODO: OpenACC: The dialect is going to change in the near future to
10061002
// have these be on a different operation, so when that changes, we
10071003
// probably need to change these here.
@@ -1042,12 +1038,13 @@ class OpenACCClauseCIREmitter final
10421038
auto recipe =
10431039
OpenACCRecipeBuilder<mlir::acc::FirstprivateRecipeOp>(cgf,
10441040
builder)
1045-
.getOrCreateRecipe(cgf.getContext(), varExpr, allocaDecl,
1046-
varRecipe.InitFromTemporary,
1047-
OpenACCReductionOperator::Invalid,
1048-
Decl::castToDeclContext(cgf.curFuncDecl),
1049-
opInfo.baseType,
1050-
firstPrivateOp.getResult());
1041+
.getOrCreateRecipe(
1042+
cgf.getContext(), varExpr, varRecipe.AllocaDecl,
1043+
varRecipe.InitExpr, varRecipe.InitFromTemporary,
1044+
OpenACCReductionOperator::Invalid,
1045+
Decl::castToDeclContext(cgf.curFuncDecl), opInfo.origType,
1046+
opInfo.bounds.size(), opInfo.boundTypes, opInfo.baseType,
1047+
firstPrivateOp.getResult());
10511048

10521049
// TODO: OpenACC: The dialect is going to change in the near future to
10531050
// have these be on a different operation, so when that changes, we
@@ -1089,11 +1086,13 @@ class OpenACCClauseCIREmitter final
10891086

10901087
auto recipe =
10911088
OpenACCRecipeBuilder<mlir::acc::ReductionRecipeOp>(cgf, builder)
1092-
.getOrCreateRecipe(cgf.getContext(), varExpr, allocaDecl,
1093-
/*temporary=*/nullptr,
1094-
clause.getReductionOp(),
1095-
Decl::castToDeclContext(cgf.curFuncDecl),
1096-
opInfo.baseType, reductionOp.getResult());
1089+
.getOrCreateRecipe(
1090+
cgf.getContext(), varExpr, varRecipe.AllocaDecl,
1091+
varRecipe.InitExpr,
1092+
/*temporary=*/nullptr, clause.getReductionOp(),
1093+
Decl::castToDeclContext(cgf.curFuncDecl), opInfo.origType,
1094+
opInfo.bounds.size(), opInfo.boundTypes, opInfo.baseType,
1095+
reductionOp.getResult());
10971096

10981097
operation.addReduction(builder.getContext(), reductionOp, recipe);
10991098
}

0 commit comments

Comments
 (0)