[OpenMP 6.0] Parse/Sema support for reduction over private variable with reduction clause. #129938
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-modules @llvm/pr-subscribers-clang Author: CHANDRA GHALE (chandraghale) ChangesInitial Parse/Sema support for reduction over private variable with reduction clause.
Patch is 195.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/129938.diff 13 Files Affected:
diff --git a/clang/include/clang/AST/OpenMPClause.h b/clang/include/clang/AST/OpenMPClause.h
index 154ecfbaa4418..51d08205850d6 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -3678,6 +3678,9 @@ class OMPReductionClause final
/// Name of custom operator.
DeclarationNameInfo NameInfo;
+ /// Private variable reduction flag
+ llvm::SmallVector<bool, 8> IsPrivateVarReductionFlags;
+
/// Build clause with number of variables \a N.
///
/// \param StartLoc Starting location of the clause.
@@ -3854,6 +3857,7 @@ class OMPReductionClause final
/// region with this clause.
/// \param PostUpdate Expression that must be executed after exit from the
/// OpenMP region with this clause.
+ /// \pram IsPrivateVarReduction array for private variable reduction flags
static OMPReductionClause *
Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation LParenLoc,
SourceLocation ModifierLoc, SourceLocation ColonLoc,
@@ -3863,7 +3867,7 @@ class OMPReductionClause final
ArrayRef<Expr *> LHSExprs, ArrayRef<Expr *> RHSExprs,
ArrayRef<Expr *> ReductionOps, ArrayRef<Expr *> CopyOps,
ArrayRef<Expr *> CopyArrayTemps, ArrayRef<Expr *> CopyArrayElems,
- Stmt *PreInit, Expr *PostUpdate);
+ Stmt *PreInit, Expr *PostUpdate, ArrayRef<bool> IsPrivateVarReduction);
/// Creates an empty clause with the place for \a N variables.
///
@@ -3889,6 +3893,16 @@ class OMPReductionClause final
/// Gets the nested name specifier.
NestedNameSpecifierLoc getQualifierLoc() const { return QualifierLoc; }
+ /// Get all private variable reduction flags
+ ArrayRef<bool> getPrivateVariableReductionFlags() const {
+ return IsPrivateVarReductionFlags;
+ }
+
+ //// Set private variable reduction flags
+ void setPrivateVariableReductionFlags(ArrayRef<bool> Flags) {
+ std::copy(Flags.begin(), Flags.end(), IsPrivateVarReductionFlags.begin());
+ }
+
using helper_expr_iterator = MutableArrayRef<Expr *>::iterator;
using helper_expr_const_iterator = ArrayRef<const Expr *>::iterator;
using helper_expr_range = llvm::iterator_range<helper_expr_iterator>;
diff --git a/clang/include/clang/Basic/OpenMPKinds.def b/clang/include/clang/Basic/OpenMPKinds.def
index 76a861f416fd5..ea01fe6386ada 100644
--- a/clang/include/clang/Basic/OpenMPKinds.def
+++ b/clang/include/clang/Basic/OpenMPKinds.def
@@ -71,6 +71,9 @@
#ifndef OPENMP_REDUCTION_MODIFIER
#define OPENMP_REDUCTION_MODIFIER(Name)
#endif
+#ifndef OPENMP_ORIGINAL_SHARING_MODIFIER
+#define OPENMP_ORIGINAL_SHARING_MODIFIER(Name)
+#endif
#ifndef OPENMP_ADJUST_ARGS_KIND
#define OPENMP_ADJUST_ARGS_KIND(Name)
#endif
@@ -202,6 +205,11 @@ OPENMP_REDUCTION_MODIFIER(default)
OPENMP_REDUCTION_MODIFIER(inscan)
OPENMP_REDUCTION_MODIFIER(task)
+// OpenMP 6.0 modifiers for 'reduction' clause.
+OPENMP_ORIGINAL_SHARING_MODIFIER(shared)
+OPENMP_ORIGINAL_SHARING_MODIFIER(private)
+OPENMP_ORIGINAL_SHARING_MODIFIER(default)
+
// Adjust-op kinds for the 'adjust_args' clause.
OPENMP_ADJUST_ARGS_KIND(nothing)
OPENMP_ADJUST_ARGS_KIND(need_device_ptr)
@@ -233,6 +241,7 @@ OPENMP_DOACROSS_MODIFIER(source_omp_cur_iteration)
#undef OPENMP_ADJUST_ARGS_KIND
#undef OPENMP_REDUCTION_MODIFIER
#undef OPENMP_DEVICE_MODIFIER
+#undef OPENMP_ORIGINAL_SHARING_MODIFIER
#undef OPENMP_ORDER_KIND
#undef OPENMP_ORDER_MODIFIER
#undef OPENMP_LASTPRIVATE_KIND
diff --git a/clang/include/clang/Basic/OpenMPKinds.h b/clang/include/clang/Basic/OpenMPKinds.h
index e80bce34a97e0..6ca9f9c550285 100644
--- a/clang/include/clang/Basic/OpenMPKinds.h
+++ b/clang/include/clang/Basic/OpenMPKinds.h
@@ -190,6 +190,13 @@ enum OpenMPReductionClauseModifier {
OMPC_REDUCTION_unknown,
};
+/// OpenMP 6.0 original sharing modifiers
+enum OpenMPOriginalSharingModifier {
+#define OPENMP_ORIGINAL_SHARING_MODIFIER(Name) OMPC_ORIGINAL_SHARING_##Name,
+#include "clang/Basic/OpenMPKinds.def"
+ OMPC_ORIGINAL_SHARING_unknown,
+};
+
/// OpenMP adjust-op kinds for 'adjust_args' clause.
enum OpenMPAdjustArgsOpKind {
#define OPENMP_ADJUST_ARGS_KIND(Name) OMPC_ADJUST_ARGS_##Name,
diff --git a/clang/include/clang/Sema/SemaOpenMP.h b/clang/include/clang/Sema/SemaOpenMP.h
index 64f0cfa0676af..90768e763894d 100644
--- a/clang/include/clang/Sema/SemaOpenMP.h
+++ b/clang/include/clang/Sema/SemaOpenMP.h
@@ -1142,6 +1142,7 @@ class SemaOpenMP : public SemaBase {
DeclarationNameInfo ReductionOrMapperId;
int ExtraModifier = -1; ///< Additional modifier for linear, map, depend or
///< lastprivate clause.
+ int OriginalSharingModifier = 0; // Default is shared
SmallVector<OpenMPMapModifierKind, NumberOfOMPMapClauseModifiers>
MapTypeModifiers;
SmallVector<SourceLocation, NumberOfOMPMapClauseModifiers>
@@ -1151,6 +1152,7 @@ class SemaOpenMP : public SemaBase {
SmallVector<SourceLocation, NumberOfOMPMotionModifiers> MotionModifiersLoc;
bool IsMapTypeImplicit = false;
SourceLocation ExtraModifierLoc;
+ SourceLocation OriginalSharingModifierLoc;
SourceLocation OmpAllMemoryLoc;
SourceLocation
StepModifierLoc; /// 'step' modifier location for linear clause
@@ -1213,7 +1215,9 @@ class SemaOpenMP : public SemaBase {
SourceLocation ModifierLoc, SourceLocation ColonLoc,
SourceLocation EndLoc, CXXScopeSpec &ReductionIdScopeSpec,
const DeclarationNameInfo &ReductionId,
- ArrayRef<Expr *> UnresolvedReductions = {});
+ ArrayRef<Expr *> UnresolvedReductions = {},
+ OpenMPOriginalSharingModifier OriginalShareModifier =
+ OMPC_ORIGINAL_SHARING_default);
/// Called on well-formed 'task_reduction' clause.
OMPClause *ActOnOpenMPTaskReductionClause(
ArrayRef<Expr *> VarList, SourceLocation StartLoc,
diff --git a/clang/lib/AST/OpenMPClause.cpp b/clang/lib/AST/OpenMPClause.cpp
index 424cab3a7de35..df4f0550e0a98 100644
--- a/clang/lib/AST/OpenMPClause.cpp
+++ b/clang/lib/AST/OpenMPClause.cpp
@@ -797,7 +797,7 @@ OMPReductionClause *OMPReductionClause::Create(
ArrayRef<Expr *> Privates, ArrayRef<Expr *> LHSExprs,
ArrayRef<Expr *> RHSExprs, ArrayRef<Expr *> ReductionOps,
ArrayRef<Expr *> CopyOps, ArrayRef<Expr *> CopyArrayTemps,
- ArrayRef<Expr *> CopyArrayElems, Stmt *PreInit, Expr *PostUpdate) {
+ ArrayRef<Expr *> CopyArrayElems, Stmt *PreInit, Expr *PostUpdate,ArrayRef<bool> IsPrivateVarReduction) {
void *Mem = C.Allocate(totalSizeToAlloc<Expr *>(
(Modifier == OMPC_REDUCTION_inscan ? 8 : 5) * VL.size()));
auto *Clause = new (Mem)
@@ -810,6 +810,7 @@ OMPReductionClause *OMPReductionClause::Create(
Clause->setReductionOps(ReductionOps);
Clause->setPreInitStmt(PreInit);
Clause->setPostUpdateExpr(PostUpdate);
+ Clause->setPrivateVariableReductionFlags(IsPrivateVarReduction);
if (Modifier == OMPC_REDUCTION_inscan) {
Clause->setInscanCopyOps(CopyOps);
Clause->setInscanCopyArrayTemps(CopyArrayTemps);
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 42e6aac681c1c..3b95eb01a06c9 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -4668,6 +4668,33 @@ bool Parser::ParseOpenMPVarList(OpenMPDirectiveKind DKind,
assert(Tok.is(tok::comma) && "Expected comma.");
(void)ConsumeToken();
}
+ // Handle original(private / shared) Modifier
+ if (Kind == OMPC_reduction && getLangOpts().OpenMP >= 60 &&
+ Tok.is(tok::identifier) && PP.getSpelling(Tok) == "original" &&
+ NextToken().is(tok::l_paren)) {
+ // Parse original(private) modifier.
+ ConsumeToken();
+ BalancedDelimiterTracker ParenT(*this, tok::l_paren, tok::r_paren);
+ ParenT.consumeOpen();
+ if (Tok.is(tok::kw_private)) {
+ Data.OriginalSharingModifier = OMPC_ORIGINAL_SHARING_private;
+ Data.OriginalSharingModifierLoc = Tok.getLocation();
+ ConsumeToken();
+ } else if (Tok.is(tok::identifier) && (PP.getSpelling(Tok) == "shared"
+ || PP.getSpelling(Tok) == "default")) {
+ Data.OriginalSharingModifier = OMPC_ORIGINAL_SHARING_shared;
+ Data.OriginalSharingModifierLoc = Tok.getLocation();
+ ConsumeToken();
+ } else {
+ Diag(Tok.getLocation(), diag::err_expected)
+ << "'private or shared or default'";
+ SkipUntil(tok::r_paren);
+ return false;
+ }
+ ParenT.consumeClose();
+ assert(Tok.is(tok::comma) && "Expected comma.");
+ (void)ConsumeToken();
+ }
ColonProtectionRAIIObject ColonRAII(*this);
if (getLangOpts().CPlusPlus)
ParseOptionalCXXScopeSpecifier(Data.ReductionOrMapperIdScopeSpec,
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 616296027d811..ae554299a01b1 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -17366,6 +17366,7 @@ OMPClause *SemaOpenMP::ActOnOpenMPVarListClause(OpenMPClauseKind Kind,
SourceLocation EndLoc = Locs.EndLoc;
OMPClause *Res = nullptr;
int ExtraModifier = Data.ExtraModifier;
+ int OriginalSharingModifier = Data.OriginalSharingModifier;
SourceLocation ExtraModifierLoc = Data.ExtraModifierLoc;
SourceLocation ColonLoc = Data.ColonLoc;
switch (Kind) {
@@ -17391,7 +17392,8 @@ OMPClause *SemaOpenMP::ActOnOpenMPVarListClause(OpenMPClauseKind Kind,
Res = ActOnOpenMPReductionClause(
VarList, static_cast<OpenMPReductionClauseModifier>(ExtraModifier),
StartLoc, LParenLoc, ExtraModifierLoc, ColonLoc, EndLoc,
- Data.ReductionOrMapperIdScopeSpec, Data.ReductionOrMapperId);
+ Data.ReductionOrMapperIdScopeSpec, Data.ReductionOrMapperId, {},
+ static_cast<OpenMPOriginalSharingModifier>(OriginalSharingModifier));
break;
case OMPC_task_reduction:
Res = ActOnOpenMPTaskReductionClause(
@@ -18570,14 +18572,20 @@ struct ReductionData {
SmallVector<Expr *, 4> ExprPostUpdates;
/// Reduction modifier.
unsigned RedModifier = 0;
+ /// Original modifier.
+ unsigned OrigSharingModifier = 0;
+ /// Private Variable Reduction
+ SmallVector<bool, 8> IsPrivateVarReduction;
ReductionData() = delete;
/// Reserves required memory for the reduction data.
- ReductionData(unsigned Size, unsigned Modifier = 0) : RedModifier(Modifier) {
+ ReductionData(unsigned Size, unsigned Modifier = 0, unsigned OrgModifier = 0)
+ : RedModifier(Modifier), OrigSharingModifier(OrgModifier) {
Vars.reserve(Size);
Privates.reserve(Size);
LHSs.reserve(Size);
RHSs.reserve(Size);
ReductionOps.reserve(Size);
+ IsPrivateVarReduction.reserve(Size);
if (RedModifier == OMPC_REDUCTION_inscan) {
InscanCopyOps.reserve(Size);
InscanCopyArrayTemps.reserve(Size);
@@ -18595,6 +18603,7 @@ struct ReductionData {
LHSs.emplace_back(nullptr);
RHSs.emplace_back(nullptr);
ReductionOps.emplace_back(ReductionOp);
+ IsPrivateVarReduction.emplace_back(false);
TaskgroupDescriptors.emplace_back(nullptr);
if (RedModifier == OMPC_REDUCTION_inscan) {
InscanCopyOps.push_back(nullptr);
@@ -18605,7 +18614,7 @@ struct ReductionData {
/// Stores reduction data.
void push(Expr *Item, Expr *Private, Expr *LHS, Expr *RHS, Expr *ReductionOp,
Expr *TaskgroupDescriptor, Expr *CopyOp, Expr *CopyArrayTemp,
- Expr *CopyArrayElem) {
+ Expr *CopyArrayElem, bool IsPrivate) {
Vars.emplace_back(Item);
Privates.emplace_back(Private);
LHSs.emplace_back(LHS);
@@ -18621,6 +18630,7 @@ struct ReductionData {
CopyArrayElem == nullptr &&
"Copy operation must be used for inscan reductions only.");
}
+ IsPrivateVarReduction.emplace_back(IsPrivate);
}
};
} // namespace
@@ -18834,6 +18844,7 @@ static bool actOnOMPReductionKindClause(
FirstIter = false;
SourceLocation ELoc;
SourceRange ERange;
+ bool IsPrivate = false;
Expr *SimpleRefExpr = RefExpr;
auto Res = getPrivateItem(S, SimpleRefExpr, ELoc, ERange,
/*AllowArraySection=*/true);
@@ -18933,12 +18944,35 @@ static bool actOnOMPReductionKindClause(
reportOriginalDsa(S, Stack, D, DVar);
continue;
}
+ // OpenMP 6.0 [ 7.6.10 ]
+ // Support Reduction over private variables with reduction clause.
+ // A list item in a reduction clause can now be private in the enclosing
+ // context. For orphaned constructs it is assumed to be shared unless the
+ // original(private) modifier appears in the clause.
+ DVar = Stack->getImplicitDSA(D, true);
+ bool IsOrphaned = false;
+ OpenMPDirectiveKind CurrDir = Stack->getCurrentDirective();
+ OpenMPDirectiveKind ParentDir = Stack->getParentDirective();
+ // Check if the construct is orphaned (has no enclosing OpenMP context)
+ IsOrphaned = (ParentDir == OMPD_unknown);
+ IsPrivate =
+ ((isOpenMPPrivate(DVar.CKind) && DVar.CKind != OMPC_reduction &&
+ isOpenMPWorksharingDirective(CurrDir) &&
+ !isOpenMPParallelDirective(CurrDir) &&
+ !isOpenMPTeamsDirective(CurrDir) &&
+ !isOpenMPSimdDirective(ParentDir)) ||
+ (IsOrphaned && DVar.CKind == OMPC_unknown) ||
+ RD.OrigSharingModifier != OMPC_ORIGINAL_SHARING_shared);
+ // Disable private handling for OpenMP versions <= 5.2
+ if (S.getLangOpts().OpenMP <= 52)
+ IsPrivate = false;
// OpenMP [2.14.3.6, Restrictions, p.1]
// A list item that appears in a reduction clause of a worksharing
// construct must be shared in the parallel regions to which any of the
// worksharing regions arising from the worksharing construct bind.
- if (isOpenMPWorksharingDirective(CurrDir) &&
+
+ if (!IsPrivate && isOpenMPWorksharingDirective(CurrDir) &&
!isOpenMPParallelDirective(CurrDir) &&
!isOpenMPTeamsDirective(CurrDir)) {
DVar = Stack->getImplicitDSA(D, true);
@@ -19434,7 +19468,7 @@ static bool actOnOMPReductionKindClause(
}
RD.push(VarsExpr, PrivateDRE, LHSDRE, RHSDRE, ReductionOp.get(),
TaskgroupDescriptor, CopyOpRes.get(), TempArrayRes.get(),
- TempArrayElem.get());
+ TempArrayElem.get(), IsPrivate);
}
return RD.Vars.empty();
}
@@ -19444,7 +19478,8 @@ OMPClause *SemaOpenMP::ActOnOpenMPReductionClause(
SourceLocation StartLoc, SourceLocation LParenLoc,
SourceLocation ModifierLoc, SourceLocation ColonLoc, SourceLocation EndLoc,
CXXScopeSpec &ReductionIdScopeSpec, const DeclarationNameInfo &ReductionId,
- ArrayRef<Expr *> UnresolvedReductions) {
+ ArrayRef<Expr *> UnresolvedReductions,
+ OpenMPOriginalSharingModifier OriginalSharingMod) {
if (ModifierLoc.isValid() && Modifier == OMPC_REDUCTION_unknown) {
Diag(LParenLoc, diag::err_omp_unexpected_clause_value)
<< getListOfPossibleValues(OMPC_reduction, /*First=*/0,
@@ -19466,8 +19501,7 @@ OMPClause *SemaOpenMP::ActOnOpenMPReductionClause(
Diag(ModifierLoc, diag::err_omp_wrong_inscan_reduction);
return nullptr;
}
-
- ReductionData RD(VarList.size(), Modifier);
+ ReductionData RD(VarList.size(), Modifier, OriginalSharingMod);
if (actOnOMPReductionKindClause(SemaRef, DSAStack, OMPC_reduction, VarList,
StartLoc, LParenLoc, ColonLoc, EndLoc,
ReductionIdScopeSpec, ReductionId,
@@ -19481,7 +19515,7 @@ OMPClause *SemaOpenMP::ActOnOpenMPReductionClause(
RD.Privates, RD.LHSs, RD.RHSs, RD.ReductionOps, RD.InscanCopyOps,
RD.InscanCopyArrayTemps, RD.InscanCopyArrayElems,
buildPreInits(getASTContext(), RD.ExprCaptures),
- buildPostUpdate(SemaRef, RD.ExprPostUpdates));
+ buildPostUpdate(SemaRef, RD.ExprPostUpdates), RD.IsPrivateVarReduction);
}
OMPClause *SemaOpenMP::ActOnOpenMPTaskReductionClause(
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 4a40df6399f64..fc01ac6097d8c 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -158,8 +158,8 @@ using llvm::BitstreamCursor;
// ChainedASTReaderListener implementation
//===----------------------------------------------------------------------===//
-bool
-ChainedASTReaderListener::ReadFullVersionInformation(StringRef FullVersion) {
+bool ChainedASTReaderListener::ReadFullVersionInformation(
+ StringRef FullVersion) {
return First->ReadFullVersionInformation(FullVersion) ||
Second->ReadFullVersionInformation(FullVersion);
}
@@ -199,9 +199,8 @@ bool ChainedASTReaderListener::ReadDiagnosticOptions(
Second->ReadDiagnosticOptions(DiagOpts, ModuleFilename, Complain);
}
-bool
-ChainedASTReaderListener::ReadFileSystemOptions(const FileSystemOptions &FSOpts,
- bool Complain) {
+bool ChainedASTReaderListener::ReadFileSystemOptions(
+ const FileSystemOptions &FSOpts, bool Complain) {
return First->ReadFileSystemOptions(FSOpts, Complain) ||
Second->ReadFileSystemOptions(FSOpts, Complain);
}
@@ -237,7 +236,7 @@ bool ChainedASTReaderListener::needsInputFileVisitation() {
bool ChainedASTReaderListener::needsSystemInputFileVisitation() {
return First->needsSystemInputFileVisitation() ||
- Second->needsSystemInputFileVisitation();
+ Second->needsSystemInputFileVisitation();
}
void ChainedASTReaderListener::visitModuleFile(StringRef Filename,
@@ -246,8 +245,7 @@ void ChainedASTReaderListener::visitModuleFile(StringRef Filename,
Second->visitModuleFile(Filename, Kind);
}
-bool ChainedASTReaderListener::visitInputFile(StringRef Filename,
- bool isSystem,
+bool ChainedASTReaderListener::visitInputFile(StringRef Filename, bool isSystem,
bool isOverridden,
bool isExplicitModule) {
bool Continue = false;
@@ -263,7 +261,7 @@ bool ChainedASTReaderListener::visitInputFile(StringRef Filename,
}
void ChainedASTReaderListener::readModuleFileExtension(
- const ModuleFileExtensionMetadata &Metadata) {
+ const ModuleFileExtensionMetadata &Metadata) {
First->readModuleFileExtension(Metadata);
Second->readModuleFileExtension(Metadata);
}
@@ -317,17 +315,17 @@ static bool checkLanguageOptions(const LangOptions &LangOpts,
return true; \
}
-#define COMPATIBLE_LANGOPT(Name, Bits, Default, Description) \
- if (!AllowCompatibleDifferences) \
- LANGOPT(Name, Bits, Default, Description)
+#define COMPATIBLE_LANGOPT(Name, Bits, Default, Description) \
+ if (!AllowCompatibleDifferences) \
+ LANGOPT(Name, Bits, Default, Description)
-#define COMPATIBLE_ENUM_LANGOPT(Name, Bits, Default, Description) \
- if (!AllowCompatibleDifferences) \
- ENUM_LANGOPT(Name, Bits, Default, Description)
+#define COMPATIBLE_ENUM_LANGOPT(Name, Bits, Default, Description) \
+ if (!AllowCompatibleDifferences) \
+ ENUM_LANGOPT(Name, Bits, Default, Description)
-#define COMPATIBLE_VALUE_LANGOPT(Name, Bits, Default, Description) \
- if (!AllowCompatibleDifferences) \
- VALUE_LANGOPT(Name, Bits, Default, Description)
+#define COMPATIBLE_VALUE_LANGOPT(Name, Bits, Default, Description) \
+ if (!AllowCompatibleDifferences) \
+ VALUE_LANGOPT(Name, Bits, Default, Description)
#define BENIGN_LANGOPT(Name, Bits, Default, Description)
#define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description)
@@ -422,8 +420,8 @@ static bool checkTargetOptions(const TargetOptions &TargetOpts,
// Compare feature sets.
SmallVector<StringRef, 4> ExistingFeatures(
- ExistingTargetOpts.FeaturesAsWritten.begin(),
- ExistingTargetOpts.FeaturesAsWritten.end());
+ ExistingTargetOpts.FeaturesAsWritten.begin(),
+ ExistingTargetOpts.FeaturesAsWritten.end());
SmallVector<StringRef, 4> ReadFeatures(TargetOpts.FeaturesAsWritten.begin(),...
[truncated]
|
| @@ -3678,6 +3678,9 @@ class OMPReductionClause final | |||
| /// Name of custom operator. | |||
| DeclarationNameInfo NameInfo; | |||
|
|
|||
| /// Private variable reduction flag | |||
| llvm::SmallVector<bool, 8> IsPrivateVarReductionFlags; | |||
There was a problem hiding this comment.
No SmallVector is allowed here, use tail-allocated storage for extra members (llvm::TrailingObjects)
clang/lib/Parse/ParseOpenMP.cpp
Outdated
| return false; | ||
| } | ||
| ParenT.consumeClose(); | ||
| assert(Tok.is(tok::comma) && "Expected comma."); |
There was a problem hiding this comment.
This should be an error, not an assertion, otherwise compiler will crash if the token is not a comma
clang/lib/Sema/SemaOpenMP.cpp
Outdated
Comment on lines
17395
to
17396
| Data.ReductionOrMapperIdScopeSpec, Data.ReductionOrMapperId, {}, | ||
| static_cast<OpenMPOriginalSharingModifier>(OriginalSharingModifier)); |
There was a problem hiding this comment.
Better to pack these members into struct already, there are too many parameters in the functions
|
| void setPrivateVariableReductionFlags(ArrayRef<bool> Flags) { | ||
| assert(Flags.size() == varlist_size() && | ||
| "Number of private flags does not match vars"); | ||
| std::copy(Flags.begin(), Flags.end(), getTrailingObjects<bool>()); |
clang/lib/Sema/SemaOpenMP.cpp
Outdated
| OpenMPDirectiveKind CurrDir = Stack->getCurrentDirective(); | ||
| OpenMPDirectiveKind ParentDir = Stack->getParentDirective(); | ||
| // Check if the construct is orphaned (has no enclosing OpenMP context) | ||
| IsOrphaned = (ParentDir == OMPD_unknown); |
Comment on lines
+18960
to
+18966
| ((isOpenMPPrivate(DVar.CKind) && DVar.CKind != OMPC_reduction && | ||
| isOpenMPWorksharingDirective(CurrDir) && | ||
| !isOpenMPParallelDirective(CurrDir) && | ||
| !isOpenMPTeamsDirective(CurrDir) && | ||
| !isOpenMPSimdDirective(ParentDir)) || | ||
| (IsOrphaned && DVar.CKind == OMPC_unknown) || | ||
| RD.OrigSharingModifier != OMPC_ORIGINAL_SHARING_shared); |
clang/lib/Sema/SemaOpenMP.cpp
Outdated
Comment on lines
18968
to
18969
| if (S.getLangOpts().OpenMP <= 52) | ||
| IsPrivate = false; |
There was a problem hiding this comment.
Shall it report an error, if DSA is private for version <= 52? If so, it should not change the value of IsPrivate here, instead it shall return IsPrivate or, if it cannot be private at all, add an assertion.
There was a problem hiding this comment.
Modified, now IsPrivate flag check is enclosed for OpenMP 5.2 and later. For versions ≤5.2, the DSA private check falls back to the existing code.
| unsigned NumFlags = Record.readInt(); | ||
| SmallVector<bool, 16> Flags; | ||
| Flags.reserve(NumFlags); | ||
| for (unsigned i = 0; i != NumFlags; ++i) |
There was a problem hiding this comment.
Suggested change
| for (unsigned i = 0; i != NumFlags; ++i) | |
| for (unsigned I : seq<unsigned>(NumFlags)) |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
|
||
| /// Get the list of help private variable reduction flags | ||
| MutableArrayRef<bool> getPrivateVariableReductionFlags() { | ||
| return MutableArrayRef<bool>(getTrailingObjects<bool>(), varlist_size()); |
There was a problem hiding this comment.
Suggested change
| return MutableArrayRef<bool>(getTrailingObjects<bool>(), varlist_size()); | |
| return MutableArrayRef(getTrailingObjects<bool>(), varlist_size()); |
?
| return MutableArrayRef<bool>(getTrailingObjects<bool>(), varlist_size()); | ||
| } | ||
| ArrayRef<bool> getPrivateVariableReductionFlags() const { | ||
| return ArrayRef<bool>(getTrailingObjects<bool>(), varlist_size()); |
There was a problem hiding this comment.
Suggested change
| return ArrayRef<bool>(getTrailingObjects<bool>(), varlist_size()); | |
| return ArrayRef(getTrailingObjects<bool>(), varlist_size()); |
| @@ -3854,6 +3890,7 @@ class OMPReductionClause final | |||
| /// region with this clause. | |||
| /// \param PostUpdate Expression that must be executed after exit from the | |||
| /// OpenMP region with this clause. | |||
| /// \pram IsPrivateVarReduction array for private variable reduction flags | |||
There was a problem hiding this comment.
Suggested change
| /// \pram IsPrivateVarReduction array for private variable reduction flags | |
| /// \param IsPrivateVarReduction array for private variable reduction flags |
| struct OpenMPReductionClauseModifiers { | ||
| int ExtraModifier; | ||
| int OriginalSharingModifier; | ||
| OpenMPReductionClauseModifiers(int extra, int original) |
There was a problem hiding this comment.
Suggested change
| OpenMPReductionClauseModifiers(int extra, int original) | |
| OpenMPReductionClauseModifiers(int Extra, int Original) |
|
Thank you for the review. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/18592 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/12975 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/18108 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/9406 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/9993 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Initial Parse/Sema support for reduction over private variable with reduction clause.
Section 7.6.10 in in OpenMP 6.0 spec.