Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,7 @@ class OpenACCClauseCIREmitter final
/*temporary=*/nullptr, OpenACCReductionOperator::Invalid,
Decl::castToDeclContext(cgf.curFuncDecl), opInfo.origType,
opInfo.bounds.size(), opInfo.boundTypes, opInfo.baseType,
privateOp);
privateOp, /*reductionCombinerRecipe=*/{});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
privateOp, /*reductionCombinerRecipe=*/{});
privateOp, /*reductionCombinerRecipes=*/{});

// TODO: OpenACC: The dialect is going to change in the near future to
// have these be on a different operation, so when that changes, we
// probably need to change these here.
Expand Down Expand Up @@ -1046,7 +1046,7 @@ class OpenACCClauseCIREmitter final
OpenACCReductionOperator::Invalid,
Decl::castToDeclContext(cgf.curFuncDecl), opInfo.origType,
opInfo.bounds.size(), opInfo.boundTypes, opInfo.baseType,
firstPrivateOp);
firstPrivateOp, /*reductionCombinerRecipe=*/{});

// TODO: OpenACC: The dialect is going to change in the near future to
// have these be on a different operation, so when that changes, we
Expand Down Expand Up @@ -1088,7 +1088,7 @@ class OpenACCClauseCIREmitter final
/*temporary=*/nullptr, clause.getReductionOp(),
Decl::castToDeclContext(cgf.curFuncDecl), opInfo.origType,
opInfo.bounds.size(), opInfo.boundTypes, opInfo.baseType,
reductionOp);
reductionOp, varRecipe.CombinerRecipes);

operation.addReduction(builder.getContext(), reductionOp, recipe);
}
Expand Down
130 changes: 127 additions & 3 deletions clang/lib/CIR/CodeGen/CIRGenOpenACCRecipe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -527,16 +527,140 @@ void OpenACCRecipeBuilderBase::createFirstprivateRecipeCopy(
// doesn't restore it aftewards.
void OpenACCRecipeBuilderBase::createReductionRecipeCombiner(
mlir::Location loc, mlir::Location locEnd, mlir::Value mainOp,
mlir::acc::ReductionRecipeOp recipe, size_t numBounds) {
mlir::acc::ReductionRecipeOp recipe, size_t numBounds, QualType origType,
llvm::ArrayRef<OpenACCReductionRecipe::CombinerRecipe> combinerRecipes) {
mlir::Block *block =
createRecipeBlock(recipe.getCombinerRegion(), mainOp.getType(), loc,
numBounds, /*isInit=*/false);
builder.setInsertionPointToEnd(&recipe.getCombinerRegion().back());
CIRGenFunction::LexicalScope ls(cgf, loc, block);

mlir::BlockArgument lhsArg = block->getArgument(0);
mlir::Value lhsArg = block->getArgument(0);
mlir::Value rhsArg = block->getArgument(1);
llvm::MutableArrayRef<mlir::BlockArgument> boundsRange =
block->getArguments().drop_front(2);

if (llvm::any_of(combinerRecipes, [](auto &r) { return r.Op == nullptr; })) {
cgf.cgm.errorNYI(loc, "OpenACC Reduction combiner not generated");
mlir::acc::YieldOp::create(builder, locEnd, block->getArgument(0));
return;
}

// apply the bounds so that we can get our bounds emitted correctly.
for (mlir::BlockArgument boundArg : llvm::reverse(boundsRange))
std::tie(lhsArg, rhsArg) =
createBoundsLoop(lhsArg, rhsArg, boundArg, loc, /*inverse=*/false);

// Emitter for when we know this isn't a struct or array we have to loop
// through. This should work for the 'field' once the get-element call has
// been made.
auto emitSingleCombiner =
[&](mlir::Value lhsArg, mlir::Value rhsArg,
const OpenACCReductionRecipe::CombinerRecipe &combiner) {
mlir::Type elementTy =
mlir::cast<cir::PointerType>(lhsArg.getType()).getPointee();
CIRGenFunction::DeclMapRevertingRAII declMapRAIILhs{cgf, combiner.LHS};
cgf.setAddrOfLocalVar(
combiner.LHS, Address{lhsArg, elementTy,
cgf.getContext().getDeclAlign(combiner.LHS)});
CIRGenFunction::DeclMapRevertingRAII declMapRAIIRhs{cgf, combiner.RHS};
cgf.setAddrOfLocalVar(
combiner.RHS, Address{rhsArg, elementTy,
cgf.getContext().getDeclAlign(combiner.RHS)});

[[maybe_unused]] mlir::LogicalResult stmtRes =
cgf.emitStmt(combiner.Op, /*useCurrentScope=*/true);
};

// Emitter for when we know this is either a non-array or element of an array
// (which also shouldn't be an array type?). This function should generate the
// loop to do this on each individual array or struct element (if necessary).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the comment. If we know this is a non-array, what does the part about a loop to do this on each individual array element mean?

auto emitCombiner = [&](mlir::Value lhsArg, mlir::Value rhsArg, QualType Ty) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
auto emitCombiner = [&](mlir::Value lhsArg, mlir::Value rhsArg, QualType Ty) {
auto emitCombiner = [&](mlir::Value lhsArg, mlir::Value rhsArg, QualType ty) {

if (const auto *RD = Ty->getAsRecordDecl()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (const auto *RD = Ty->getAsRecordDecl()) {
if (const auto *rd = Ty->getAsRecordDecl()) {

if (combinerRecipes.size() == 1 &&
cgf.getContext().hasSameType(Ty, combinerRecipes[0].LHS->getType())) {
// If this is a 'top level' operator on the type we can just emit this
// as a simple one.
emitSingleCombiner(lhsArg, rhsArg, combinerRecipes[0]);
} else {
// else we have to handle each individual field after after a
// get-element.
for (const auto &[field, combiner] :
llvm::zip_equal(RD->fields(), combinerRecipes)) {
mlir::Type fieldType = cgf.convertType(field->getType());
auto fieldPtr = cir::PointerType::get(fieldType);

mlir::Value lhsField = builder.createGetMember(
loc, fieldPtr, lhsArg, field->getName(), field->getFieldIndex());
mlir::Value rhsField = builder.createGetMember(
loc, fieldPtr, rhsArg, field->getName(), field->getFieldIndex());

emitSingleCombiner(lhsField, rhsField, combiner);
}
}

} else {
// if this is a single-thing (because we should know this isn't an array,
// as Sema wouldn't let us get here), we can just do a normal emit call.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe assert that it's not an array?

emitSingleCombiner(lhsArg, rhsArg, combinerRecipes[0]);
}
};

if (const auto *cat = cgf.getContext().getAsConstantArrayType(origType)) {
// If we're in an array, we have to emit the combiner for each element of
// the array.
auto itrTy = mlir::cast<cir::IntType>(cgf.PtrDiffTy);
auto itrPtrTy = cir::PointerType::get(itrTy);

mlir::Value zero =
builder.getConstInt(loc, mlir::cast<cir::IntType>(cgf.PtrDiffTy), 0);
mlir::Value itr =
cir::AllocaOp::create(builder, loc, itrPtrTy, itrTy, "itr",
cgf.cgm.getSize(cgf.getPointerAlign()));
builder.CIRBaseBuilderTy::createStore(loc, zero, itr);

builder.setInsertionPointAfter(builder.createFor(
loc,
/*condBuilder=*/
[&](mlir::OpBuilder &b, mlir::Location loc) {
auto loadItr = cir::LoadOp::create(builder, loc, {itr});
mlir::Value arraySize = builder.getConstInt(
loc, mlir::cast<cir::IntType>(cgf.PtrDiffTy), cat->getZExtSize());
auto cmp = builder.createCompare(loc, cir::CmpOpKind::lt, loadItr,
arraySize);
builder.createCondition(cmp);
},
/*bodyBuilder=*/
[&](mlir::OpBuilder &b, mlir::Location loc) {
auto loadItr = cir::LoadOp::create(builder, loc, {itr});
auto lhsElt = builder.getArrayElement(
loc, loc, lhsArg, cgf.convertType(cat->getElementType()), loadItr,
/*shouldDecay=*/true);
auto rhsElt = builder.getArrayElement(
loc, loc, rhsArg, cgf.convertType(cat->getElementType()), loadItr,
/*shouldDecay=*/true);

emitCombiner(lhsElt, rhsElt, cat->getElementType());
builder.createYield(loc);
},
/*stepBuilder=*/
[&](mlir::OpBuilder &b, mlir::Location loc) {
auto loadItr = cir::LoadOp::create(builder, loc, {itr});
auto inc = cir::UnaryOp::create(builder, loc, loadItr.getType(),
cir::UnaryOpKind::Inc, loadItr);
builder.CIRBaseBuilderTy::createStore(loc, inc, itr);
builder.createYield(loc);
}));

mlir::acc::YieldOp::create(builder, locEnd, lhsArg);
} else if (origType->isArrayType()) {
cgf.cgm.errorNYI(loc,
"OpenACC Reduction combiner non-constant array recipe");
} else {
emitCombiner(lhsArg, rhsArg, origType);
}

builder.setInsertionPointToEnd(&recipe.getCombinerRegion().back());
mlir::acc::YieldOp::create(builder, locEnd, block->getArgument(0));
}

} // namespace clang::CIRGen
15 changes: 9 additions & 6 deletions clang/lib/CIR/CodeGen/CIRGenOpenACCRecipe.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ class OpenACCRecipeBuilderBase {
// that this function is not 'insertion point' clean, in that it alters the
// insertion point to be inside of the 'combiner' section of the recipe, but
// doesn't restore it aftewards.
void createReductionRecipeCombiner(mlir::Location loc, mlir::Location locEnd,
mlir::Value mainOp,
mlir::acc::ReductionRecipeOp recipe,
size_t numBounds);
void createReductionRecipeCombiner(
mlir::Location loc, mlir::Location locEnd, mlir::Value mainOp,
mlir::acc::ReductionRecipeOp recipe, size_t numBounds, QualType origType,
llvm::ArrayRef<OpenACCReductionRecipe::CombinerRecipe> combinerRecipes);

void createInitRecipe(mlir::Location loc, mlir::Location locEnd,
SourceRange exprRange, mlir::Value mainOp,
Expand Down Expand Up @@ -169,7 +169,9 @@ class OpenACCRecipeBuilder : OpenACCRecipeBuilderBase {
const Expr *varRef, const VarDecl *varRecipe, const VarDecl *temporary,
OpenACCReductionOperator reductionOp, DeclContext *dc, QualType origType,
size_t numBounds, llvm::ArrayRef<QualType> boundTypes, QualType baseType,
mlir::Value mainOp) {
mlir::Value mainOp,
llvm::ArrayRef<OpenACCReductionRecipe::CombinerRecipe>
reductionCombinerRecipes) {
assert(!varRecipe->getType()->isSpecificBuiltinType(
BuiltinType::ArraySection) &&
"array section shouldn't make it to recipe creation");
Expand Down Expand Up @@ -208,7 +210,8 @@ class OpenACCRecipeBuilder : OpenACCRecipeBuilderBase {
createInitRecipe(loc, locEnd, varRef->getSourceRange(), mainOp,
recipe.getInitRegion(), numBounds, boundTypes, varRecipe,
origType, /*emitInitExpr=*/true);
createReductionRecipeCombiner(loc, locEnd, mainOp, recipe, numBounds);
createReductionRecipeCombiner(loc, locEnd, mainOp, recipe, numBounds,
origType, reductionCombinerRecipes);
} else {
static_assert(std::is_same_v<RecipeTy, mlir::acc::FirstprivateRecipeOp>);
createInitRecipe(loc, locEnd, varRef->getSourceRange(), mainOp,
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaOpenACCClause.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1924,7 +1924,7 @@ bool SemaOpenACC::CheckReductionVarType(Expr *VarExpr) {
// off here. This will result in CurType being the actual 'type' of the
// expression, which is what we are looking to check.
QualType CurType = isa<ArraySectionExpr>(VarExpr)
? ArraySectionExpr::getBaseOriginalType(VarExpr)
? cast<ArraySectionExpr>(VarExpr)->getElementType()
: VarExpr->getType();

// This can happen when we have a dependent type in an array element that the
Expand Down
Loading