Skip to content

[OpenACC] Reduction 'init' lowering for all-ones/least/largest #156535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 3, 2025
Merged

[OpenACC] Reduction 'init' lowering for all-ones/least/largest #156535

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a176d3e
[OpenACC] Reduction 'init' lowering for all-ones/least/largest
erichkeane Sep 2, 2025
e6fce23
set floats to all something reasonable/nonsense
erichkeane Sep 2, 2025
a40f666
Do a bunch of review stuff Andy suggested:
erichkeane Sep 2, 2025
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
124 changes: 102 additions & 22 deletions clang/lib/Sema/SemaOpenACC.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2589,26 +2589,63 @@ SemaOpenACC::ActOnOpenACCAsteriskSizeExpr(SourceLocation AsteriskLoc) {
return BuildOpenACCAsteriskSizeExpr(AsteriskLoc);
}

/// Loops through a type and generates an appropriate InitListExpr to generate
/// type initialization.
static Expr *GenerateReductionInitRecipeExpr(ASTContext &Context,
SourceRange ExprRange,
QualType Ty) {
namespace {
enum class InitKind { One, AllOnes, Least, Largest };
llvm::APFloat getInitFloatValue(ASTContext &Context, InitKind IK, QualType Ty) {
switch (IK) {
case InitKind::One:
return llvm::APFloat::getOne(Context.getFloatTypeSemantics(Ty));
case InitKind::AllOnes:
return llvm::APFloat::getAllOnesValue(Context.getFloatTypeSemantics(Ty));
case InitKind::Least:
return llvm::APFloat::getSmallestNormalized(
Copy link
Contributor

@andykaylor andykaylor Sep 2, 2025

Choose a reason for hiding this comment

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

Are you sure you don't want llvm::APFloat::getLargest(Context.getFloatTypeSemantics(Ty), /*Negative=*/true); here? I think getSmallestNormalized is going to return a value near zero.

Copy link
Collaborator Author

@erichkeane erichkeane Sep 2, 2025

Choose a reason for hiding this comment

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

Ah! Good to know, thank you!

Context.getFloatTypeSemantics(Ty), /*Negative=*/true);
case InitKind::Largest:
return llvm::APFloat::getLargest(Context.getFloatTypeSemantics(Ty));
break;
}
llvm_unreachable("unknown init kind");
}

llvm::APInt getInitIntValue(ASTContext &Context, InitKind IK, QualType Ty) {
switch (IK) {
case InitKind::One:
return llvm::APInt(Context.getIntWidth(Ty), 1);
case InitKind::AllOnes:
return llvm::APInt::getAllOnes(Context.getIntWidth(Ty));
case InitKind::Least:
if (Ty->isSignedIntegerOrEnumerationType())
return llvm::APInt::getSignedMinValue(Context.getIntWidth(Ty));
return llvm::APInt::getMinValue(Context.getIntWidth(Ty));
case InitKind::Largest:
if (Ty->isSignedIntegerOrEnumerationType())
return llvm::APInt::getSignedMaxValue(Context.getIntWidth(Ty));
return llvm::APInt::getMaxValue(Context.getIntWidth(Ty));
break;
}
llvm_unreachable("unknown init kind");
}

/// Loops through a type and generates an appropriate InitListExpr to
/// generate type initialization.
Expr *GenerateReductionInitRecipeExpr(ASTContext &Context,
SourceRange ExprRange, QualType Ty,
InitKind IK) {
Ty = Ty.getCanonicalType();
llvm::SmallVector<Expr *> Exprs;

if (const RecordDecl *RD = Ty->getAsRecordDecl()) {
for (auto *F : RD->fields()) {
if (Expr *NewExpr =
GenerateReductionInitRecipeExpr(Context, ExprRange, F->getType()))
if (Expr *NewExpr = GenerateReductionInitRecipeExpr(Context, ExprRange,
F->getType(), IK))
Exprs.push_back(NewExpr);
else
return nullptr;
}
} else if (const ConstantArrayType *AT = Context.getAsConstantArrayType(Ty)) {
for (uint64_t Idx = 0; Idx < AT->getZExtSize(); ++Idx) {
if (Expr *NewExpr = GenerateReductionInitRecipeExpr(Context, ExprRange,
AT->getElementType()))
if (Expr *NewExpr = GenerateReductionInitRecipeExpr(
Context, ExprRange, AT->getElementType(), IK))
Exprs.push_back(NewExpr);
else
return nullptr;
Expand All @@ -2627,16 +2664,40 @@ static Expr *GenerateReductionInitRecipeExpr(ASTContext &Context,
} else {
assert(Ty->isScalarType());

// TODO: OpenACC: This currently only works for '1', but we need to figure
// out a way to do least/largest/all-1s.
if (Ty->isFloatingType()) {
Exprs.push_back(FloatingLiteral::Create(
Context, llvm::APFloat::getOne(Context.getFloatTypeSemantics(Ty)),
/*isExact=*/true, Ty, ExprRange.getBegin()));
if (const auto *Cplx = Ty->getAs<ComplexType>()) {
// we can get here in error cases, so make sure we generate something that
// will work if we find ourselves wanting to enable this.

QualType EltTy = Cplx->getElementType();
if (EltTy->isFloatingType()) {
Exprs.push_back(FloatingLiteral::Create(
Context, getInitFloatValue(Context, IK, EltTy),
/*isExact=*/true, EltTy, ExprRange.getBegin()));
Exprs.push_back(FloatingLiteral::Create(
Copy link
Contributor

@andykaylor andykaylor Sep 2, 2025

Choose a reason for hiding this comment

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

This probably isn't what you want. For example, for InitKind::One this will generate 1+1i but the identity value for complex multiplication is 1 + 0i. Maybe best to leave it as an error-producing case.

Copy link
Collaborator Author

@erichkeane erichkeane Sep 2, 2025

Choose a reason for hiding this comment

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

Could you guide me as to what the right answer is for them? I could definitely split these up into separate functions, but I'd love to know what the '0', '1', All-Ones, Least and Largest should be here?

I think I'd one day soon want to enable complex here.

Copy link
Contributor

@andykaylor andykaylor Sep 2, 2025

Choose a reason for hiding this comment

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

I'm having a hard time picturing the use case for all-ones with floating point, but I suppose literally all ones for both components makes as much sense as anything. You could define starting points for minimum and maximum, but in general complex numbers can't be ordered in any meaningful sense. There is no < operator for complex values, for instance. When I googled this, I saw a few suggestions that you could compare the magnitude of the values, for which, largest or least for both component values would work. So I guess, InitKind::One is the only one for which what you did here wouldn't work. Trying to answer this question, on the other hand, clarified for me why the OpenACC standard wouldn't generally want to accept complex values.

Copy link
Collaborator Author

@erichkeane erichkeane Sep 2, 2025

Choose a reason for hiding this comment

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

Ah, hmm.. ok then, I think leaving complex as ill-formed is the best idea.

I DID just realize my test before was wrong, and I HAVEN'T been disallowing complex, so I'm going to go through and do that as a RAC patch.

Copy link
Contributor

@andykaylor andykaylor Sep 2, 2025

Choose a reason for hiding this comment

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

Oops. Magnitude is an absolute value, so I guess you'd need {0, 0} for the LEAST case. Anyway, it's probably just a bad idea to allow it at all.

Copy link
Collaborator Author

@erichkeane erichkeane Sep 2, 2025

Choose a reason for hiding this comment

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

I could probably make complex legal for some operators (+/*?), but it doesn't seem worth the effort until someone asks for it. In the meantime, I've made sure it is ill-formed always, here: 7a0dfb1

Context, getInitFloatValue(Context, IK, EltTy),
/*isExact=*/true, EltTy, ExprRange.getBegin()));
} else {
Exprs.push_back(
IntegerLiteral::Create(Context, getInitIntValue(Context, IK, EltTy),
EltTy, ExprRange.getBegin()));
Exprs.push_back(
IntegerLiteral::Create(Context, getInitIntValue(Context, IK, EltTy),
EltTy, ExprRange.getBegin()));
}

} else if (Ty->isFloatingType()) {
Exprs.push_back(
FloatingLiteral::Create(Context, getInitFloatValue(Context, IK, Ty),
/*isExact=*/true, Ty, ExprRange.getBegin()));
} else if (Ty->isBooleanType()) {
Exprs.push_back(CXXBoolLiteralExpr::Create(Context,
(IK == InitKind::One ||
IK == InitKind::AllOnes ||
IK == InitKind::Largest),
Ty, ExprRange.getBegin()));
} else {
Exprs.push_back(IntegerLiteral::Create(
Context, llvm::APInt(Context.getTypeSize(Ty), 1), Ty,
ExprRange.getBegin()));
Context, getInitIntValue(Context, IK, Ty), Ty, ExprRange.getBegin()));
}
}

Expand All @@ -2646,6 +2707,8 @@ static Expr *GenerateReductionInitRecipeExpr(ASTContext &Context,
return InitExpr;
}

} // namespace

std::pair<VarDecl *, VarDecl *>
SemaOpenACC::CreateInitRecipe(OpenACCClauseKind CK,
OpenACCReductionOperator ReductionOperator,
Expand Down Expand Up @@ -2796,20 +2859,37 @@ SemaOpenACC::CreateInitRecipe(OpenACCClauseKind CK,
// are used for code generation, we can just ignore/not bother doing any
// initialization here.
break;
case OpenACCReductionOperator::Max:
case OpenACCReductionOperator::Min:
case OpenACCReductionOperator::BitwiseAnd:
// TODO: OpenACC: figure out init for these.
case OpenACCReductionOperator::Max: {
Expr *InitExpr = GenerateReductionInitRecipeExpr(
getASTContext(), VarExpr->getSourceRange(), VarTy, InitKind::Least);

Init = FinishValueInit(InitExpr);
break;
}
case OpenACCReductionOperator::Min: {
Expr *InitExpr = GenerateReductionInitRecipeExpr(
getASTContext(), VarExpr->getSourceRange(), VarTy,
InitKind::Largest);

Init = FinishValueInit(InitExpr);
break;
}
case OpenACCReductionOperator::BitwiseAnd: {
Expr *InitExpr = GenerateReductionInitRecipeExpr(
getASTContext(), VarExpr->getSourceRange(), VarTy,
InitKind::AllOnes);

Init = FinishValueInit(InitExpr);
break;
}
case OpenACCReductionOperator::Multiplication:
case OpenACCReductionOperator::And: {
// '&&' initializes every field to 1. However, we need to loop through
// every field/element and generate an initializer for each of the
// elements.

Expr *InitExpr = GenerateReductionInitRecipeExpr(
getASTContext(), VarExpr->getSourceRange(), VarTy);
getASTContext(), VarExpr->getSourceRange(), VarTy, InitKind::One);

Init = FinishValueInit(InitExpr);
break;
Expand Down
Loading
Loading