Skip to content

Commit 19043b2

Browse files
authored
[OpenMP] Report errors when construct decomposition fails (#167568)
Store the list of errors in the ConsstructDecomposition class in addition to the broken up output. This not used in flang yet, because the splitting happens at a time when diagnostic messages can no longer be emitted. Use unit tests to test this instead.
1 parent c0ac0c4 commit 19043b2

File tree

2 files changed

+173
-36
lines changed

2 files changed

+173
-36
lines changed

llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h

Lines changed: 69 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,13 @@ find_unique(Container &&container, Predicate &&pred) {
6868

6969
namespace tomp {
7070

71+
enum struct ErrorCode : int {
72+
NoLeafAllowing, // No leaf that allows this clause
73+
NoLeafPrivatizing, // No leaf that has a privatizing clause
74+
InvalidDirNameMod, // Invalid directive name modifier
75+
RedModNotApplied, // Reduction modifier not applied
76+
};
77+
7178
// ClauseType: Either an instance of ClauseT, or a type derived from ClauseT.
7279
// This is the clause representation in the code using this infrastructure.
7380
//
@@ -114,10 +121,16 @@ struct ConstructDecompositionT {
114121
}
115122

116123
tomp::ListT<DirectiveWithClauses<ClauseType>> output;
124+
llvm::SmallVector<std::pair<const ClauseType *, ErrorCode>> errors;
117125

118126
private:
119127
bool split();
120128

129+
bool error(const ClauseTy *node, ErrorCode ec) {
130+
errors.emplace_back(node, ec);
131+
return false;
132+
}
133+
121134
struct LeafReprInternal {
122135
llvm::omp::Directive id = llvm::omp::Directive::OMPD_unknown;
123136
tomp::type::ListT<const ClauseTy *> clauses;
@@ -456,10 +469,9 @@ bool ConstructDecompositionT<C, H>::applyClause(Specific &&specific,
456469
// S Some clauses are permitted only on a single leaf construct of the
457470
// S combined or composite construct, in which case the effect is as if
458471
// S the clause is applied to that specific construct. (p339, 31-33)
459-
if (applyToUnique(node))
460-
return true;
461-
462-
return false;
472+
if (!applyToUnique(node))
473+
return error(node, ErrorCode::NoLeafAllowing);
474+
return true;
463475
}
464476

465477
// --- Specific clauses -----------------------------------------------
@@ -487,7 +499,9 @@ bool ConstructDecompositionT<C, H>::applyClause(
487499
});
488500
});
489501

490-
return applied;
502+
if (!applied)
503+
return error(node, ErrorCode::NoLeafPrivatizing);
504+
return true;
491505
}
492506

493507
// COLLAPSE
@@ -501,7 +515,9 @@ template <typename C, typename H>
501515
bool ConstructDecompositionT<C, H>::applyClause(
502516
const tomp::clause::CollapseT<TypeTy, IdTy, ExprTy> &clause,
503517
const ClauseTy *node) {
504-
return applyToInnermost(node);
518+
if (!applyToInnermost(node))
519+
return error(node, ErrorCode::NoLeafAllowing);
520+
return true;
505521
}
506522

507523
// DEFAULT
@@ -516,7 +532,9 @@ bool ConstructDecompositionT<C, H>::applyClause(
516532
const tomp::clause::DefaultT<TypeTy, IdTy, ExprTy> &clause,
517533
const ClauseTy *node) {
518534
// [5.2:340:31]
519-
return applyToAll(node);
535+
if (!applyToAll(node))
536+
return error(node, ErrorCode::NoLeafAllowing);
537+
return true;
520538
}
521539

522540
// FIRSTPRIVATE
@@ -644,7 +662,9 @@ bool ConstructDecompositionT<C, H>::applyClause(
644662
applied = true;
645663
}
646664

647-
return applied;
665+
if (!applied)
666+
return error(node, ErrorCode::NoLeafAllowing);
667+
return true;
648668
}
649669

650670
// IF
@@ -679,10 +699,12 @@ bool ConstructDecompositionT<C, H>::applyClause(
679699
hasDir->clauses.push_back(unmodified);
680700
return true;
681701
}
682-
return false;
702+
return error(node, ErrorCode::InvalidDirNameMod);
683703
}
684704

685-
return applyToAll(node);
705+
if (!applyToAll(node))
706+
return error(node, ErrorCode::NoLeafAllowing);
707+
return true;
686708
}
687709

688710
// LASTPRIVATE
@@ -708,12 +730,9 @@ template <typename C, typename H>
708730
bool ConstructDecompositionT<C, H>::applyClause(
709731
const tomp::clause::LastprivateT<TypeTy, IdTy, ExprTy> &clause,
710732
const ClauseTy *node) {
711-
bool applied = false;
712-
713733
// [5.2:340:21]
714-
applied = applyToAll(node);
715-
if (!applied)
716-
return false;
734+
if (!applyToAll(node))
735+
return error(node, ErrorCode::NoLeafAllowing);
717736

718737
auto inFirstprivate = [&](const ObjectTy &object) {
719738
if (ClauseSet *set = findClausesWith(object)) {
@@ -739,7 +758,6 @@ bool ConstructDecompositionT<C, H>::applyClause(
739758
llvm::omp::Clause::OMPC_shared,
740759
tomp::clause::SharedT<TypeTy, IdTy, ExprTy>{/*List=*/sharedObjects});
741760
dirParallel->clauses.push_back(shared);
742-
applied = true;
743761
}
744762

745763
// [5.2:340:24]
@@ -748,7 +766,6 @@ bool ConstructDecompositionT<C, H>::applyClause(
748766
llvm::omp::Clause::OMPC_shared,
749767
tomp::clause::SharedT<TypeTy, IdTy, ExprTy>{/*List=*/sharedObjects});
750768
dirTeams->clauses.push_back(shared);
751-
applied = true;
752769
}
753770
}
754771

@@ -772,11 +789,10 @@ bool ConstructDecompositionT<C, H>::applyClause(
772789
/*Mapper=*/std::nullopt, /*Iterator=*/std::nullopt,
773790
/*LocatorList=*/std::move(tofrom)}});
774791
dirTarget->clauses.push_back(map);
775-
applied = true;
776792
}
777793
}
778794

779-
return applied;
795+
return true;
780796
}
781797

782798
// LINEAR
@@ -802,7 +818,7 @@ bool ConstructDecompositionT<C, H>::applyClause(
802818
const ClauseTy *node) {
803819
// [5.2:341:15.1]
804820
if (!applyToInnermost(node))
805-
return false;
821+
return error(node, ErrorCode::NoLeafAllowing);
806822

807823
// [5.2:341:15.2], [5.2:341:19]
808824
auto dirSimd = findDirective(llvm::omp::Directive::OMPD_simd);
@@ -847,24 +863,29 @@ template <typename C, typename H>
847863
bool ConstructDecompositionT<C, H>::applyClause(
848864
const tomp::clause::NowaitT<TypeTy, IdTy, ExprTy> &clause,
849865
const ClauseTy *node) {
850-
return applyToOutermost(node);
866+
if (!applyToOutermost(node))
867+
return error(node, ErrorCode::NoLeafAllowing);
868+
return true;
851869
}
852870

853871
// OMPX_ATTRIBUTE
854872
template <typename C, typename H>
855873
bool ConstructDecompositionT<C, H>::applyClause(
856874
const tomp::clause::OmpxAttributeT<TypeTy, IdTy, ExprTy> &clause,
857875
const ClauseTy *node) {
858-
// ERROR: no leaf that allows clause
859-
return applyToAll(node);
876+
if (!applyToAll(node))
877+
return error(node, ErrorCode::NoLeafAllowing);
878+
return true;
860879
}
861880

862881
// OMPX_BARE
863882
template <typename C, typename H>
864883
bool ConstructDecompositionT<C, H>::applyClause(
865884
const tomp::clause::OmpxBareT<TypeTy, IdTy, ExprTy> &clause,
866885
const ClauseTy *node) {
867-
return applyToOutermost(node);
886+
if (!applyToOutermost(node))
887+
return error(node, ErrorCode::NoLeafAllowing);
888+
return true;
868889
}
869890

870891
// ORDER
@@ -879,7 +900,9 @@ bool ConstructDecompositionT<C, H>::applyClause(
879900
const tomp::clause::OrderT<TypeTy, IdTy, ExprTy> &clause,
880901
const ClauseTy *node) {
881902
// [5.2:340:31]
882-
return applyToAll(node);
903+
if (!applyToAll(node))
904+
return error(node, ErrorCode::NoLeafAllowing);
905+
return true;
883906
}
884907

885908
// PRIVATE
@@ -894,7 +917,9 @@ template <typename C, typename H>
894917
bool ConstructDecompositionT<C, H>::applyClause(
895918
const tomp::clause::PrivateT<TypeTy, IdTy, ExprTy> &clause,
896919
const ClauseTy *node) {
897-
return applyToInnermost(node);
920+
if (!applyToInnermost(node))
921+
return error(node, ErrorCode::NoLeafAllowing);
922+
return true;
898923
}
899924

900925
// REDUCTION
@@ -996,31 +1021,37 @@ bool ConstructDecompositionT<C, H>::applyClause(
9961021
/*List=*/objects}});
9971022

9981023
ReductionModifier effective = modifier.value_or(ReductionModifier::Default);
999-
bool effectiveApplied = false;
1024+
bool modifierApplied = false;
1025+
bool allowingLeaf = false;
10001026
// Walk over the leaf constructs starting from the innermost, and apply
10011027
// the clause as required by the spec.
10021028
for (auto &leaf : llvm::reverse(leafs)) {
10031029
if (!llvm::omp::isAllowedClauseForDirective(leaf.id, node->id, version))
10041030
continue;
1031+
// Found a leaf that allows this clause. Keep track of this for better
1032+
// error reporting.
1033+
allowingLeaf = true;
10051034
if (!applyToParallel && &leaf == dirParallel)
10061035
continue;
10071036
if (!applyToTeams && &leaf == dirTeams)
10081037
continue;
10091038
// Some form of the clause will be applied past this point.
1010-
if (isValidModifier(leaf.id, effective, effectiveApplied)) {
1039+
if (isValidModifier(leaf.id, effective, modifierApplied)) {
10111040
// Apply clause with modifier.
10121041
leaf.clauses.push_back(node);
1013-
effectiveApplied = true;
1042+
modifierApplied = true;
10141043
} else {
10151044
// Apply clause without modifier.
10161045
leaf.clauses.push_back(unmodified);
10171046
}
10181047
// The modifier must be applied to some construct.
1019-
applied = effectiveApplied;
1048+
applied = modifierApplied;
10201049
}
10211050

1051+
if (!allowingLeaf)
1052+
return error(node, ErrorCode::NoLeafAllowing);
10221053
if (!applied)
1023-
return false;
1054+
return error(node, ErrorCode::RedModNotApplied);
10241055

10251056
tomp::ObjectListT<IdTy, ExprTy> sharedObjects;
10261057
llvm::transform(objects, std::back_inserter(sharedObjects),
@@ -1067,11 +1098,10 @@ bool ConstructDecompositionT<C, H>::applyClause(
10671098
/*LocatorList=*/std::move(tofrom)}});
10681099

10691100
dirTarget->clauses.push_back(map);
1070-
applied = true;
10711101
}
10721102
}
10731103

1074-
return applied;
1104+
return true;
10751105
}
10761106

10771107
// SHARED
@@ -1086,7 +1116,9 @@ bool ConstructDecompositionT<C, H>::applyClause(
10861116
const tomp::clause::SharedT<TypeTy, IdTy, ExprTy> &clause,
10871117
const ClauseTy *node) {
10881118
// [5.2:340:31]
1089-
return applyToAll(node);
1119+
if (!applyToAll(node))
1120+
return error(node, ErrorCode::NoLeafAllowing);
1121+
return true;
10901122
}
10911123

10921124
// THREAD_LIMIT
@@ -1101,7 +1133,9 @@ bool ConstructDecompositionT<C, H>::applyClause(
11011133
const tomp::clause::ThreadLimitT<TypeTy, IdTy, ExprTy> &clause,
11021134
const ClauseTy *node) {
11031135
// [5.2:340:31]
1104-
return applyToAll(node);
1136+
if (!applyToAll(node))
1137+
return error(node, ErrorCode::NoLeafAllowing);
1138+
return true;
11051139
}
11061140

11071141
// --- Splitting ------------------------------------------------------

0 commit comments

Comments
 (0)