diff --git a/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h b/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h index c8eebbf42a68e..36b49e69650d8 100644 --- a/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h +++ b/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h @@ -68,6 +68,13 @@ find_unique(Container &&container, Predicate &&pred) { namespace tomp { +enum struct ErrorCode : int { + NoLeafAllowing, // No leaf that allows this clause + NoLeafPrivatizing, // No leaf that has a privatizing clause + InvalidDirNameMod, // Invalid directive name modifier + RedModNotApplied, // Reduction modifier not applied +}; + // ClauseType: Either an instance of ClauseT, or a type derived from ClauseT. // This is the clause representation in the code using this infrastructure. // @@ -114,10 +121,16 @@ struct ConstructDecompositionT { } tomp::ListT> output; + llvm::SmallVector> errors; private: bool split(); + bool error(const ClauseTy *node, ErrorCode ec) { + errors.emplace_back(node, ec); + return false; + } + struct LeafReprInternal { llvm::omp::Directive id = llvm::omp::Directive::OMPD_unknown; tomp::type::ListT clauses; @@ -456,10 +469,9 @@ bool ConstructDecompositionT::applyClause(Specific &&specific, // S Some clauses are permitted only on a single leaf construct of the // S combined or composite construct, in which case the effect is as if // S the clause is applied to that specific construct. (p339, 31-33) - if (applyToUnique(node)) - return true; - - return false; + if (!applyToUnique(node)) + return error(node, ErrorCode::NoLeafAllowing); + return true; } // --- Specific clauses ----------------------------------------------- @@ -487,7 +499,9 @@ bool ConstructDecompositionT::applyClause( }); }); - return applied; + if (!applied) + return error(node, ErrorCode::NoLeafPrivatizing); + return true; } // COLLAPSE @@ -501,7 +515,9 @@ template bool ConstructDecompositionT::applyClause( const tomp::clause::CollapseT &clause, const ClauseTy *node) { - return applyToInnermost(node); + if (!applyToInnermost(node)) + return error(node, ErrorCode::NoLeafAllowing); + return true; } // DEFAULT @@ -516,7 +532,9 @@ bool ConstructDecompositionT::applyClause( const tomp::clause::DefaultT &clause, const ClauseTy *node) { // [5.2:340:31] - return applyToAll(node); + if (!applyToAll(node)) + return error(node, ErrorCode::NoLeafAllowing); + return true; } // FIRSTPRIVATE @@ -644,7 +662,9 @@ bool ConstructDecompositionT::applyClause( applied = true; } - return applied; + if (!applied) + return error(node, ErrorCode::NoLeafAllowing); + return true; } // IF @@ -679,10 +699,12 @@ bool ConstructDecompositionT::applyClause( hasDir->clauses.push_back(unmodified); return true; } - return false; + return error(node, ErrorCode::InvalidDirNameMod); } - return applyToAll(node); + if (!applyToAll(node)) + return error(node, ErrorCode::NoLeafAllowing); + return true; } // LASTPRIVATE @@ -708,12 +730,9 @@ template bool ConstructDecompositionT::applyClause( const tomp::clause::LastprivateT &clause, const ClauseTy *node) { - bool applied = false; - // [5.2:340:21] - applied = applyToAll(node); - if (!applied) - return false; + if (!applyToAll(node)) + return error(node, ErrorCode::NoLeafAllowing); auto inFirstprivate = [&](const ObjectTy &object) { if (ClauseSet *set = findClausesWith(object)) { @@ -739,7 +758,6 @@ bool ConstructDecompositionT::applyClause( llvm::omp::Clause::OMPC_shared, tomp::clause::SharedT{/*List=*/sharedObjects}); dirParallel->clauses.push_back(shared); - applied = true; } // [5.2:340:24] @@ -748,7 +766,6 @@ bool ConstructDecompositionT::applyClause( llvm::omp::Clause::OMPC_shared, tomp::clause::SharedT{/*List=*/sharedObjects}); dirTeams->clauses.push_back(shared); - applied = true; } } @@ -772,11 +789,10 @@ bool ConstructDecompositionT::applyClause( /*Mapper=*/std::nullopt, /*Iterator=*/std::nullopt, /*LocatorList=*/std::move(tofrom)}}); dirTarget->clauses.push_back(map); - applied = true; } } - return applied; + return true; } // LINEAR @@ -802,7 +818,7 @@ bool ConstructDecompositionT::applyClause( const ClauseTy *node) { // [5.2:341:15.1] if (!applyToInnermost(node)) - return false; + return error(node, ErrorCode::NoLeafAllowing); // [5.2:341:15.2], [5.2:341:19] auto dirSimd = findDirective(llvm::omp::Directive::OMPD_simd); @@ -847,7 +863,9 @@ template bool ConstructDecompositionT::applyClause( const tomp::clause::NowaitT &clause, const ClauseTy *node) { - return applyToOutermost(node); + if (!applyToOutermost(node)) + return error(node, ErrorCode::NoLeafAllowing); + return true; } // OMPX_ATTRIBUTE @@ -855,8 +873,9 @@ template bool ConstructDecompositionT::applyClause( const tomp::clause::OmpxAttributeT &clause, const ClauseTy *node) { - // ERROR: no leaf that allows clause - return applyToAll(node); + if (!applyToAll(node)) + return error(node, ErrorCode::NoLeafAllowing); + return true; } // OMPX_BARE @@ -864,7 +883,9 @@ template bool ConstructDecompositionT::applyClause( const tomp::clause::OmpxBareT &clause, const ClauseTy *node) { - return applyToOutermost(node); + if (!applyToOutermost(node)) + return error(node, ErrorCode::NoLeafAllowing); + return true; } // ORDER @@ -879,7 +900,9 @@ bool ConstructDecompositionT::applyClause( const tomp::clause::OrderT &clause, const ClauseTy *node) { // [5.2:340:31] - return applyToAll(node); + if (!applyToAll(node)) + return error(node, ErrorCode::NoLeafAllowing); + return true; } // PRIVATE @@ -894,7 +917,9 @@ template bool ConstructDecompositionT::applyClause( const tomp::clause::PrivateT &clause, const ClauseTy *node) { - return applyToInnermost(node); + if (!applyToInnermost(node)) + return error(node, ErrorCode::NoLeafAllowing); + return true; } // REDUCTION @@ -996,31 +1021,37 @@ bool ConstructDecompositionT::applyClause( /*List=*/objects}}); ReductionModifier effective = modifier.value_or(ReductionModifier::Default); - bool effectiveApplied = false; + bool modifierApplied = false; + bool allowingLeaf = false; // Walk over the leaf constructs starting from the innermost, and apply // the clause as required by the spec. for (auto &leaf : llvm::reverse(leafs)) { if (!llvm::omp::isAllowedClauseForDirective(leaf.id, node->id, version)) continue; + // Found a leaf that allows this clause. Keep track of this for better + // error reporting. + allowingLeaf = true; if (!applyToParallel && &leaf == dirParallel) continue; if (!applyToTeams && &leaf == dirTeams) continue; // Some form of the clause will be applied past this point. - if (isValidModifier(leaf.id, effective, effectiveApplied)) { + if (isValidModifier(leaf.id, effective, modifierApplied)) { // Apply clause with modifier. leaf.clauses.push_back(node); - effectiveApplied = true; + modifierApplied = true; } else { // Apply clause without modifier. leaf.clauses.push_back(unmodified); } // The modifier must be applied to some construct. - applied = effectiveApplied; + applied = modifierApplied; } + if (!allowingLeaf) + return error(node, ErrorCode::NoLeafAllowing); if (!applied) - return false; + return error(node, ErrorCode::RedModNotApplied); tomp::ObjectListT sharedObjects; llvm::transform(objects, std::back_inserter(sharedObjects), @@ -1067,11 +1098,10 @@ bool ConstructDecompositionT::applyClause( /*LocatorList=*/std::move(tofrom)}}); dirTarget->clauses.push_back(map); - applied = true; } } - return applied; + return true; } // SHARED @@ -1086,7 +1116,9 @@ bool ConstructDecompositionT::applyClause( const tomp::clause::SharedT &clause, const ClauseTy *node) { // [5.2:340:31] - return applyToAll(node); + if (!applyToAll(node)) + return error(node, ErrorCode::NoLeafAllowing); + return true; } // THREAD_LIMIT @@ -1101,7 +1133,9 @@ bool ConstructDecompositionT::applyClause( const tomp::clause::ThreadLimitT &clause, const ClauseTy *node) { // [5.2:340:31] - return applyToAll(node); + if (!applyToAll(node)) + return error(node, ErrorCode::NoLeafAllowing); + return true; } // --- Splitting ------------------------------------------------------ diff --git a/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp b/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp index e0c6b3904310c..23c3c4d5d192c 100644 --- a/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp +++ b/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp @@ -277,16 +277,42 @@ struct StringifyClause { std::string Str; }; +std::string stringify(const omp::Clause &C) { // + return StringifyClause(C).Str; +} + std::string stringify(const omp::DirectiveWithClauses &DWC) { std::stringstream Stream; Stream << getOpenMPDirectiveName(DWC.id, llvm::omp::FallbackVersion).str(); for (const omp::Clause &C : DWC.clauses) - Stream << ' ' << StringifyClause(C).Str; + Stream << ' ' << stringify(C); return Stream.str(); } +std::string stringify(tomp::ErrorCode E) { + switch (E) { + case tomp::ErrorCode::NoLeafAllowing: + return "no leaf that allows this clause"; + case tomp::ErrorCode::NoLeafPrivatizing: + return "no leaf with a privatizing clause"; + case tomp::ErrorCode::InvalidDirNameMod: + return "invalid directive name modifier"; + case tomp::ErrorCode::RedModNotApplied: + return "the reduction modifier cannot be applied"; + } + return "unrecognized error code " + std::to_string(llvm::to_underlying(E)); +} + +std::string stringify(std::pair &ER) { + std::stringstream Stream; + + Stream << "error while applying '" << stringify(*ER.first) + << "': " << stringify(ER.second); + return Stream.str(); +} + // --- Tests ---------------------------------------------------------- namespace red { @@ -1109,4 +1135,81 @@ TEST_F(OpenMPDecompositionTest, Misc1) { std::string Dir0 = stringify(Dec.output[0]); ASSERT_EQ(Dir0, "simd linear(, , (x)) lastprivate(, (x))"); } + +// --- Failure/error reporting tests + +TEST_F(OpenMPDecompositionTest, Error1) { + // "parallel for at(compilation)" is invalid because the "at" clause + // does not apply to either "parallel" or "for". + + omp::List Clauses{ + {OMPC_at, omp::clause::At{omp::clause::At::ActionTime::Compilation}}, + }; + + omp::ConstructDecomposition Dec(AnyVersion, Helper, OMPD_parallel_for, + Clauses); + ASSERT_EQ(Dec.errors.size(), 1u); + std::string Err0 = stringify(Dec.errors[0]); + ASSERT_EQ(Err0, + "error while applying 'at(0)': no leaf that allows this clause"); +} + +TEST_F(OpenMPDecompositionTest, Error2) { + // "parallel loop allocate(x) private(x)" is invalid because "allocate" + // can only be applied to "parallel", while "private" is applied to "loop". + // This violates the requirement that the leaf with an "allocate" also has + // a privatizing clause. + + omp::Object x{"x"}; + + omp::List Clauses{ + {OMPC_allocate, omp::clause::Allocate{{std::nullopt, std::nullopt, {x}}}}, + {OMPC_private, omp::clause::Private{{x}}}, + }; + + omp::ConstructDecomposition Dec(AnyVersion, Helper, OMPD_parallel_loop, + Clauses); + ASSERT_EQ(Dec.errors.size(), 1u); + std::string Err0 = stringify(Dec.errors[0]); + ASSERT_EQ(Err0, "error while applying 'allocate(, , (x))': no leaf with a " + "privatizing clause"); +} + +TEST_F(OpenMPDecompositionTest, Error3) { + // "parallel for if(target: e)" is invalid because the "target" directive- + // name-modifier does not refer to a constituent directive. + + omp::ExprTy e; + + omp::List Clauses{ + {OMPC_if, omp::clause::If{{llvm::omp::Directive::OMPD_target, e}}}, + }; + + omp::ConstructDecomposition Dec(AnyVersion, Helper, OMPD_parallel_for, + Clauses); + ASSERT_EQ(Dec.errors.size(), 1u); + std::string Err0 = stringify(Dec.errors[0]); + ASSERT_EQ(Err0, "error while applying 'if(target, expr)': invalid directive " + "name modifier"); +} + +TEST_F(OpenMPDecompositionTest, Error4) { + // "masked taskloop reduction(+, task: x)" is invalid because the "task" + // modifier can only be applied to "parallel" or a worksharing directive. + + omp::Object x{"x"}; + auto Add = red::makeOp(omp::clause::DefinedOperator::IntrinsicOperator::Add); + auto TaskMod = omp::clause::Reduction::ReductionModifier::Task; + + omp::List Clauses{ + {OMPC_reduction, omp::clause::Reduction{{TaskMod, {Add}, {x}}}}, + }; + + omp::ConstructDecomposition Dec(AnyVersion, Helper, OMPD_masked_taskloop, + Clauses); + ASSERT_EQ(Dec.errors.size(), 1u); + std::string Err0 = stringify(Dec.errors[0]); + ASSERT_EQ(Err0, "error while applying 'reduction(2, (3), (x))': the " + "reduction modifier cannot be applied"); +} } // namespace