Skip to content

Commit 08c98ba

Browse files
authored
[NFC][OpenACC] Reorder 'recipe' generation to be lexical (#160585)
It was noticed on a previous patch that I could have emitted recipes in lexical order instead of reverse order, which would improve the readability of a lot of tests. This patch implements that, and changes all of the required test.
1 parent 12bc084 commit 08c98ba

File tree

40 files changed

+9907
-10493
lines changed

40 files changed

+9907
-10493
lines changed

clang/lib/CIR/CodeGen/CIRGenFunction.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1716,6 +1716,10 @@ class CIRGenFunction : public CIRGenTypeCache {
17161716
~ActiveOpenACCLoopRAII() { cgf.activeLoopOp = oldLoopOp; }
17171717
};
17181718

1719+
// Keep track of the last place we inserted a 'recipe' so that we can insert
1720+
// the next one in lexical order.
1721+
mlir::OpBuilder::InsertPoint lastRecipeLocation;
1722+
17191723
public:
17201724
// Helper type used to store the list of important information for a 'data'
17211725
// clause variable, or a 'cache' variable reference.
@@ -1733,6 +1737,7 @@ class CIRGenFunction : public CIRGenTypeCache {
17331737
// can use to properly set the alloca section.
17341738
llvm::SmallVector<QualType> boundTypes;
17351739
};
1740+
17361741
// Gets the collection of info required to lower and OpenACC clause or cache
17371742
// construct variable reference.
17381743
OpenACCDataOperandInfo getOpenACCDataOperandInfo(const Expr *e);

clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ class OpenACCClauseCIREmitter final
5353
template <typename FriendOpTy> friend class OpenACCClauseCIREmitter;
5454

5555
OpTy &operation;
56+
mlir::OpBuilder::InsertPoint &recipeInsertLocation;
5657
CIRGen::CIRGenFunction &cgf;
5758
CIRGen::CIRGenBuilderTy &builder;
5859

@@ -148,7 +149,7 @@ class OpenACCClauseCIREmitter final
148149
mlir::OpBuilder::InsertionGuard guardCase(builder);
149150
builder.setInsertionPoint(operation.loopOp);
150151
OpenACCClauseCIREmitter<mlir::acc::LoopOp> loopEmitter{
151-
operation.loopOp, cgf, builder, dirKind, dirLoc};
152+
operation.loopOp, recipeInsertLocation, cgf, builder, dirKind, dirLoc};
152153
loopEmitter.lastDeviceTypeValues = lastDeviceTypeValues;
153154
loopEmitter.Visit(&c);
154155
}
@@ -159,7 +160,12 @@ class OpenACCClauseCIREmitter final
159160
mlir::OpBuilder::InsertionGuard guardCase(builder);
160161
builder.setInsertionPoint(operation.computeOp);
161162
OpenACCClauseCIREmitter<typename OpTy::ComputeOpTy> computeEmitter{
162-
operation.computeOp, cgf, builder, dirKind, dirLoc};
163+
operation.computeOp,
164+
recipeInsertLocation,
165+
cgf,
166+
builder,
167+
dirKind,
168+
dirLoc};
163169

164170
computeEmitter.lastDeviceTypeValues = lastDeviceTypeValues;
165171

@@ -358,11 +364,13 @@ class OpenACCClauseCIREmitter final
358364
}
359365

360366
public:
361-
OpenACCClauseCIREmitter(OpTy &operation, CIRGen::CIRGenFunction &cgf,
367+
OpenACCClauseCIREmitter(OpTy &operation,
368+
mlir::OpBuilder::InsertPoint &recipeInsertLocation,
369+
CIRGen::CIRGenFunction &cgf,
362370
CIRGen::CIRGenBuilderTy &builder,
363371
OpenACCDirectiveKind dirKind, SourceLocation dirLoc)
364-
: operation(operation), cgf(cgf), builder(builder), dirKind(dirKind),
365-
dirLoc(dirLoc) {}
372+
: operation(operation), recipeInsertLocation(recipeInsertLocation),
373+
cgf(cgf), builder(builder), dirKind(dirKind), dirLoc(dirLoc) {}
366374

367375
void VisitClause(const OpenACCClause &clause) {
368376
clauseNotImplemented(clause);
@@ -992,8 +1000,8 @@ class OpenACCClauseCIREmitter final
9921000
auto recipe =
9931001
OpenACCRecipeBuilder<mlir::acc::PrivateRecipeOp>(cgf, builder)
9941002
.getOrCreateRecipe(
995-
cgf.getContext(), varExpr, varRecipe.AllocaDecl,
996-
varRecipe.InitExpr,
1003+
cgf.getContext(), recipeInsertLocation, varExpr,
1004+
varRecipe.AllocaDecl, varRecipe.InitExpr,
9971005
/*temporary=*/nullptr, OpenACCReductionOperator::Invalid,
9981006
Decl::castToDeclContext(cgf.curFuncDecl), opInfo.origType,
9991007
opInfo.bounds.size(), opInfo.boundTypes, opInfo.baseType,
@@ -1039,8 +1047,9 @@ class OpenACCClauseCIREmitter final
10391047
OpenACCRecipeBuilder<mlir::acc::FirstprivateRecipeOp>(cgf,
10401048
builder)
10411049
.getOrCreateRecipe(
1042-
cgf.getContext(), varExpr, varRecipe.AllocaDecl,
1043-
varRecipe.InitExpr, varRecipe.InitFromTemporary,
1050+
cgf.getContext(), recipeInsertLocation, varExpr,
1051+
varRecipe.AllocaDecl, varRecipe.InitExpr,
1052+
varRecipe.InitFromTemporary,
10441053
OpenACCReductionOperator::Invalid,
10451054
Decl::castToDeclContext(cgf.curFuncDecl), opInfo.origType,
10461055
opInfo.bounds.size(), opInfo.boundTypes, opInfo.baseType,
@@ -1087,8 +1096,8 @@ class OpenACCClauseCIREmitter final
10871096
auto recipe =
10881097
OpenACCRecipeBuilder<mlir::acc::ReductionRecipeOp>(cgf, builder)
10891098
.getOrCreateRecipe(
1090-
cgf.getContext(), varExpr, varRecipe.AllocaDecl,
1091-
varRecipe.InitExpr,
1099+
cgf.getContext(), recipeInsertLocation, varExpr,
1100+
varRecipe.AllocaDecl, varRecipe.InitExpr,
10921101
/*temporary=*/nullptr, clause.getReductionOp(),
10931102
Decl::castToDeclContext(cgf.curFuncDecl), opInfo.origType,
10941103
opInfo.bounds.size(), opInfo.boundTypes, opInfo.baseType,
@@ -1108,10 +1117,13 @@ class OpenACCClauseCIREmitter final
11081117
};
11091118

11101119
template <typename OpTy>
1111-
auto makeClauseEmitter(OpTy &op, CIRGen::CIRGenFunction &cgf,
1120+
auto makeClauseEmitter(OpTy &op,
1121+
mlir::OpBuilder::InsertPoint &recipeInsertLocation,
1122+
CIRGen::CIRGenFunction &cgf,
11121123
CIRGen::CIRGenBuilderTy &builder,
11131124
OpenACCDirectiveKind dirKind, SourceLocation dirLoc) {
1114-
return OpenACCClauseCIREmitter<OpTy>(op, cgf, builder, dirKind, dirLoc);
1125+
return OpenACCClauseCIREmitter<OpTy>(op, recipeInsertLocation, cgf, builder,
1126+
dirKind, dirLoc);
11151127
}
11161128
} // namespace
11171129

@@ -1124,7 +1136,8 @@ void CIRGenFunction::emitOpenACCClauses(
11241136
// Sets insertion point before the 'op', since every new expression needs to
11251137
// be before the operation.
11261138
builder.setInsertionPoint(op);
1127-
makeClauseEmitter(op, *this, builder, dirKind, dirLoc).emitClauses(clauses);
1139+
makeClauseEmitter(op, lastRecipeLocation, *this, builder, dirKind, dirLoc)
1140+
.emitClauses(clauses);
11281141
}
11291142

11301143
#define EXPL_SPEC(N) \
@@ -1156,7 +1169,8 @@ void CIRGenFunction::emitOpenACCClauses(
11561169
// We cannot set the insertion point here and do so in the emitter, but make
11571170
// sure we reset it with the 'guard' anyway.
11581171
mlir::OpBuilder::InsertionGuard guardCase(builder);
1159-
makeClauseEmitter(inf, *this, builder, dirKind, dirLoc).emitClauses(clauses);
1172+
makeClauseEmitter(inf, lastRecipeLocation, *this, builder, dirKind, dirLoc)
1173+
.emitClauses(clauses);
11601174
}
11611175

11621176
#define EXPL_SPEC(N) \

clang/lib/CIR/CodeGen/CIRGenOpenACCRecipe.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -469,9 +469,10 @@ template <typename RecipeTy> class OpenACCRecipeBuilder {
469469
OpenACCRecipeBuilder(CIRGen::CIRGenFunction &cgf,
470470
CIRGen::CIRGenBuilderTy &builder)
471471
: cgf(cgf), builder(builder) {}
472-
RecipeTy getOrCreateRecipe(ASTContext &astCtx, const Expr *varRef,
473-
const VarDecl *varRecipe, const Expr *initExpr,
474-
const VarDecl *temporary,
472+
RecipeTy getOrCreateRecipe(ASTContext &astCtx,
473+
mlir::OpBuilder::InsertPoint &insertLocation,
474+
const Expr *varRef, const VarDecl *varRecipe,
475+
const Expr *initExpr, const VarDecl *temporary,
475476
OpenACCReductionOperator reductionOp,
476477
DeclContext *dc, QualType origType,
477478
size_t numBounds,
@@ -507,6 +508,8 @@ template <typename RecipeTy> class OpenACCRecipeBuilder {
507508
mlir::Location locEnd = cgf.cgm.getLoc(varRef->getEndLoc());
508509

509510
mlir::OpBuilder modBuilder(mod.getBodyRegion());
511+
if (insertLocation.isSet())
512+
modBuilder.restoreInsertionPoint(insertLocation);
510513
RecipeTy recipe;
511514

512515
if constexpr (std::is_same_v<RecipeTy, mlir::acc::ReductionRecipeOp>) {
@@ -515,6 +518,7 @@ template <typename RecipeTy> class OpenACCRecipeBuilder {
515518
} else {
516519
recipe = RecipeTy::create(modBuilder, loc, recipeName, mainOp.getType());
517520
}
521+
insertLocation = modBuilder.saveInsertionPoint();
518522

519523
if constexpr (std::is_same_v<RecipeTy, mlir::acc::PrivateRecipeOp>) {
520524
createPrivateInitRecipe(loc, locEnd, varRef->getSourceRange(), mainOp,

0 commit comments

Comments
 (0)