Skip to content

Commit

Permalink
[OpenMP][Part 2] Use reusable OpenMP context/traits handling
Browse files Browse the repository at this point in the history
This patch implements an almost complete handling of OpenMP
contexts/traits such that we can reuse most of the logic in Flang
through the OMPContext.{h,cpp} in llvm/Frontend/OpenMP.

All but construct SIMD specifiers, e.g., inbranch, and the device ISA
selector are define in `llvm/lib/Frontend/OpenMP/OMPKinds.def`. From
these definitions we generate the enum classes `TraitSet`,
`TraitSelector`, and `TraitProperty` as well as conversion and helper
functions in `llvm/lib/Frontend/OpenMP/OMPContext.{h,cpp}`.

The above enum classes are used in the parser, sema, and the AST
attribute. The latter is not a collection of multiple primitive variant
arguments that contain encodings via numbers and strings but instead a
tree that mirrors the `match` clause (see `struct OpenMPTraitInfo`).

The changes to the parser make it more forgiving when wrong syntax is
read and they also resulted in more specialized diagnostics. The tests
are updated and the core issues are detected as before. Here and
elsewhere this patch tries to be generic, thus we do not distinguish
what selector set, selector, or property is parsed except if they do
behave exceptionally, as for example `user={condition(EXPR)}` does.

The sema logic changed in two ways: First, the OMPDeclareVariantAttr
representation changed, as mentioned above, and the sema was adjusted to
work with the new `OpenMPTraitInfo`. Second, the matching and scoring
logic moved into `OMPContext.{h,cpp}`. It is implemented on a flat
representation of the `match` clause that is not tied to clang.
`OpenMPTraitInfo` provides a method to generate this flat structure (see
`struct VariantMatchInfo`) by computing integer score values and boolean
user conditions from the `clang::Expr` we keep for them.

The OpenMP context is now an explicit object (see `struct OMPContext`).
This is in anticipation of construct traits that need to be tracked. The
OpenMP context, as well as the `VariantMatchInfo`, are basically made up
of a set of active or respectively required traits, e.g., 'host', and an
ordered container of constructs which allows duplication. Matching and
scoring is kept as generic as possible to allow easy extension in the
future.

---

Test changes:

The messages checked in `OpenMP/declare_variant_messages.{c,cpp}` have
been auto generated to match the new warnings and notes of the parser.
The "subset" checks were reversed causing the wrong version to be
picked. The tests have been adjusted to correct this.
We do not print scores if the user did not provide one.
We print spaces to make lists in the `match` clause more legible.

Reviewers: kiranchandramohan, ABataev, RaviNarayanaswamy, gtbercea, grokos, sdmitriev, JonChesterfield, hfinkel, fghanim

Subscribers: merge_guards_bot, rampitec, mgorny, hiraditya, aheejin, fedor.sergeev, simoncook, bollu, guansong, dexonsmith, jfb, s.egerton, llvm-commits, cfe-commits

Tags: #clang, #llvm

Differential Revision: https://reviews.llvm.org/D71830
  • Loading branch information
jdoerfert committed Feb 14, 2020
1 parent 4f2cccc commit 1228d42
Show file tree
Hide file tree
Showing 31 changed files with 1,348 additions and 1,141 deletions.
1 change: 1 addition & 0 deletions clang/include/clang/AST/Attr.h
Expand Up @@ -17,6 +17,7 @@
#include "clang/AST/AttrIterator.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
#include "clang/AST/OpenMPClause.h"
#include "clang/AST/Type.h"
#include "clang/Basic/AttrKinds.h"
#include "clang/Basic/AttributeCommonInfo.h"
Expand Down
48 changes: 48 additions & 0 deletions clang/include/clang/AST/OpenMPClause.h
Expand Up @@ -31,6 +31,7 @@
#include "llvm/ADT/iterator.h"
#include "llvm/ADT/iterator_range.h"
#include "llvm/Frontend/OpenMP/OMPConstants.h"
#include "llvm/Frontend/OpenMP/OMPContext.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/TrailingObjects.h"
Expand Down Expand Up @@ -6658,6 +6659,53 @@ class OMPClausePrinter final : public OMPClauseVisitor<OMPClausePrinter> {
#include "clang/Basic/OpenMPKinds.def"
};

/// Helper data structure representing the traits in a match clause of an
/// `declare variant` or `metadirective`. The outer level is an ordered
/// collection of selector sets, each with an associated kind and an ordered
/// collection of selectors. A selector has a kind, an optional score/condition,
/// and an ordered collection of properties.
struct OMPTraitInfo {
struct OMPTraitProperty {
llvm::omp::TraitProperty Kind = llvm::omp::TraitProperty::invalid;
};
struct OMPTraitSelector {
Expr *ScoreOrCondition = nullptr;
llvm::omp::TraitSelector Kind = llvm::omp::TraitSelector::invalid;
llvm::SmallVector<OMPTraitProperty, 4> Properties;
};
struct OMPTraitSet {
llvm::omp::TraitSet Kind = llvm::omp::TraitSet::invalid;
llvm::SmallVector<OMPTraitSelector, 4> Selectors;
};

/// The outermost level of selector sets.
llvm::SmallVector<OMPTraitSet, 4> Sets;

bool anyScoreOrCondition(
llvm::function_ref<bool(Expr *&, bool /* IsScore */)> Cond) {
return llvm::any_of(Sets, [Cond](OMPTraitInfo::OMPTraitSet &Set) {
return llvm::any_of(
Set.Selectors, [Cond](OMPTraitInfo::OMPTraitSelector &Selector) {
return Cond(Selector.ScoreOrCondition,
/* IsScore */ Selector.Kind !=
llvm::omp::TraitSelector::user_condition);
});
});
}

/// Create a variant match info object from this trait info object. While the
/// former is a flat representation the actual main difference is that the
/// latter uses clang::Expr to store the score/condition while the former is
/// independent of clang. Thus, expressions and conditions are evaluated in
/// this method.
void getAsVariantMatchInfo(ASTContext &ASTCtx,
llvm::omp::VariantMatchInfo &VMI) const;

/// Print a human readable representation into \p OS.
void print(llvm::raw_ostream &OS, const PrintingPolicy &Policy) const;
};
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const OMPTraitInfo &TI);

} // namespace clang

#endif // LLVM_CLANG_AST_OPENMPCLAUSE_H
95 changes: 24 additions & 71 deletions clang/include/clang/Basic/Attr.td
Expand Up @@ -180,6 +180,27 @@ class FunctionArgument<string name, bit opt = 0, bit fake = 0> : Argument<name,
class NamedArgument<string name, bit opt = 0, bit fake = 0> : Argument<name,
opt,
fake>;

// An argument of a OMPDeclareVariantAttribute that represents the `match`
// clause of the declare variant by keeping the information (incl. nesting) in
// an OMPTraitInfo object.
//
// With some exceptions, the `match(<context-selector>)` clause looks roughly
// as follows:
// context-selector := list<selector-set>
// selector-set := <kind>={list<selector>}
// selector := <kind>([score(<const-expr>):] list<trait>)
// trait := <kind>
//
// The structure of an OMPTraitInfo object is a tree as defined below:
//
// OMPTraitInfo := {list<OMPTraitSet>}
// OMPTraitSet := {Kind, list<OMPTraitSelector>}
// OMPTraitSelector := {Kind, Expr, list<OMPTraitProperty>}
// OMPTraitProperty := {Kind}
//
class OMPTraitInfoArgument<string name> : Argument<name, 0>;

class TypeArgument<string name, bit opt = 0> : Argument<name, opt>;
class UnsignedArgument<string name, bit opt = 0> : Argument<name, opt>;
class VariadicUnsignedArgument<string name> : Argument<name, 1>;
Expand Down Expand Up @@ -3342,87 +3363,19 @@ def OMPDeclareVariant : InheritableAttr {
let Documentation = [OMPDeclareVariantDocs];
let Args = [
ExprArgument<"VariantFuncRef">,
VariadicExprArgument<"Scores">,
VariadicUnsignedArgument<"CtxSelectorSets">,
VariadicUnsignedArgument<"CtxSelectors">,
VariadicStringArgument<"ImplVendors">,
VariadicStringArgument<"DeviceKinds">
OMPTraitInfoArgument<"TraitInfos">,
];
let AdditionalMembers = [{
void printScore(raw_ostream & OS, const PrintingPolicy &Policy, unsigned I) const {
if (const Expr *E = *std::next(scores_begin(), I)) {
OS << "score(";
E->printPretty(OS, nullptr, Policy);
OS << "):";
}
}
~OMPDeclareVariantAttr() { delete traitInfos; }
void printPrettyPragma(raw_ostream & OS, const PrintingPolicy &Policy)
const {
if (const Expr *E = getVariantFuncRef()) {
OS << "(";
E->printPretty(OS, nullptr, Policy);
OS << ")";
}
// TODO: add printing of real context selectors.
OS << " match(";
int Used[OMP_CTX_SET_unknown] = {0};
for (unsigned I = 0, E = ctxSelectorSets_size(); I < E; ++I) {
auto CtxSet = static_cast<OpenMPContextSelectorSetKind>(
*std::next(ctxSelectorSets_begin(), I));
if (Used[CtxSet])
continue;
if (I > 0)
OS << ",";
switch (CtxSet) {
case OMP_CTX_SET_implementation:
OS << "implementation={";
break;
case OMP_CTX_SET_device:
OS << "device={";
break;
case OMP_CTX_SET_unknown:
llvm_unreachable("Unknown context selector set.");
}
Used[CtxSet] = 1;
for (unsigned K = I, EK = ctxSelectors_size(); K < EK; ++K) {
auto CtxSetK = static_cast<OpenMPContextSelectorSetKind>(
*std::next(ctxSelectorSets_begin(), K));
if (CtxSet != CtxSetK)
continue;
if (K != I)
OS << ",";
auto Ctx = static_cast<OpenMPContextSelectorKind>(
*std::next(ctxSelectors_begin(), K));
switch (Ctx) {
case OMP_CTX_vendor:
assert(CtxSet == OMP_CTX_SET_implementation &&
"Expected implementation context selector set.");
OS << "vendor(";
printScore(OS, Policy, K);
if (implVendors_size() > 0) {
OS << *implVendors(). begin();
for (StringRef VendorName : llvm::drop_begin(implVendors(), 1))
OS << ", " << VendorName;
}
OS << ")";
break;
case OMP_CTX_kind:
assert(CtxSet == OMP_CTX_SET_device &&
"Expected device context selector set.");
OS << "kind(";
if (deviceKinds_size() > 0) {
OS << *deviceKinds().begin();
for (StringRef KindName : llvm::drop_begin(deviceKinds(), 1))
OS << ", " << KindName;
}
OS << ")";
break;
case OMP_CTX_unknown:
llvm_unreachable("Unknown context selector.");
}
}
OS << "}";
}
traitInfos->print(OS, Policy);
OS << ")";
}
}];
Expand Down
86 changes: 62 additions & 24 deletions clang/include/clang/Basic/DiagnosticParseKinds.td
Expand Up @@ -1258,30 +1258,68 @@ def err_omp_mapper_expected_declarator : Error<
"expected declarator on 'omp declare mapper' directive">;
def err_omp_declare_variant_wrong_clause : Error<
"expected '%0' clause on 'omp declare variant' directive">;
def err_omp_declare_variant_no_ctx_selector : Error<
"expected context selector in '%0' clause on 'omp declare variant' directive">;
def err_omp_declare_variant_equal_expected : Error<
"expected '=' after '%0' context selector set name on 'omp declare variant' directive">;
def warn_omp_declare_variant_cs_name_expected : Warning<
"unknown context selector in '%0' context selector set of 'omp declare variant' directive, ignored">,
InGroup<OpenMPClauses>;
def err_omp_declare_variant_item_expected : Error<
"expected %0 in '%1' context selector of '%2' selector set of 'omp declare variant' directive">;
def err_omp_declare_variant_ctx_set_mutiple_use : Error<
"context selector set '%0' is used already in the same 'omp declare variant' directive">;
def note_omp_declare_variant_ctx_set_used_here : Note<
"previously context selector set '%0' used here">;
def err_omp_expected_comma_brace : Error<"expected '}' or ',' after '%0'">;
def err_omp_declare_variant_ctx_mutiple_use : Error<
"context trait selector '%0' is used already in the same '%1' context selector set of 'omp declare variant' directive">;
def note_omp_declare_variant_ctx_used_here : Note<
"previously context trait selector '%0' used here">;
def warn_omp_more_one_device_type_clause : Warning<
"more than one 'device_type' clause is specified">,
InGroup<OpenMPClauses>;
def err_omp_wrong_device_kind_trait : Error<
"unknown '%0' device kind trait in the 'device' context selector set, expected"
" one of 'host', 'nohost', 'cpu', 'gpu' or 'fpga'">;
def warn_omp_declare_variant_string_literal_or_identifier
: Warning<"expected identifier or string literal describing a context "
"%select{set|selector|property}0; "
"%select{set|selector|property}0 skipped">,
InGroup<OpenMPClauses>;
def note_omp_declare_variant_ctx_options
: Note<"context %select{set|selector|property}0 options are: %1">;
def warn_omp_declare_variant_expected
: Warning<"expected '%0' after the %1; '%0' assumed">,
InGroup<OpenMPClauses>;
def warn_omp_declare_variant_ctx_not_a_property
: Warning<"'%0' is not a valid context property for the context selector "
"'%1' and the context set '%2'; property ignored">,
InGroup<OpenMPClauses>;
def note_omp_declare_variant_ctx_is_a
: Note<"'%0' is a context %select{set|selector|property}1 not a context "
"%select{set|selector|property}2">;
def note_omp_declare_variant_ctx_try : Note<"try 'match(%0={%1%2})'">;
def warn_omp_declare_variant_ctx_not_a_selector
: Warning<"'%0' is not a valid context selector for the context set '%1'; "
"selector ignored">,
InGroup<OpenMPClauses>;
def warn_omp_declare_variant_ctx_not_a_set
: Warning<"'%0' is not a valid context set in a `declare variant`; set "
"ignored">,
InGroup<OpenMPClauses>;
def warn_omp_declare_variant_ctx_mutiple_use
: Warning<"the context %select{set|selector|property}0 '%1' was used "
"already in the same 'omp declare variant' directive; "
"%select{set|selector|property}0 ignored">,
InGroup<OpenMPClauses>;
def note_omp_declare_variant_ctx_used_here
: Note<"the previous context %select{set|selector|property}0 '%1' used "
"here">;
def note_omp_declare_variant_ctx_continue_here
: Note<"the ignored %select{set|selector|property}0 spans until here">;
def warn_omp_ctx_incompatible_selector_for_set
: Warning<"the context selector '%0' is not valid for the context set "
"'%1'; selector ignored">,
InGroup<OpenMPClauses>;
def note_omp_ctx_compatible_set_for_selector
: Note<"the context selector '%0' can be nested in the context set '%1'; "
"try 'match(%1={%0%select{|(property)}2})'">;
def warn_omp_ctx_selector_without_properties
: Warning<"the context selector '%0' in context set '%1' requires a "
"context property defined in parentheses; selector ignored">,
InGroup<OpenMPClauses>;
def warn_omp_ctx_incompatible_property_for_selector
: Warning<"the context property '%0' is not valid for the context selector "
"'%1' and the context set '%2'; property ignored">,
InGroup<OpenMPClauses>;
def note_omp_ctx_compatible_set_and_selector_for_property
: Note<"the context property '%0' can be nested in the context selector "
"'%1' which is nested in the context set '%2'; try "
"'match(%2={%1(%0)})'">;
def warn_omp_ctx_incompatible_score_for_property
: Warning<"the context selector '%0' in the context set '%1' cannot have a "
"score ('%2'); score ignored">,
InGroup<OpenMPClauses>;
def warn_omp_more_one_device_type_clause
: Warning<"more than one 'device_type' clause is specified">,
InGroup<OpenMPClauses>;

// Pragma loop support.
def err_pragma_loop_missing_argument : Error<
Expand Down
6 changes: 6 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Expand Up @@ -9910,6 +9910,12 @@ def warn_omp_declare_target_after_first_use : Warning<
InGroup<OpenMPTarget>;
def err_omp_declare_variant_incompat_attributes : Error<
"'#pragma omp declare variant' is not compatible with any target-specific attributes">;
def warn_omp_declare_variant_score_not_constant
: Warning<"score expressions in the OpenMP context selector need to be "
"constant; %0 is not and will be ignored">;
def err_omp_declare_variant_user_condition_not_constant
: Error<"the user condition in the OpenMP context selector needs to be "
"constant; %0 is not">;
def warn_omp_declare_variant_after_used : Warning<
"'#pragma omp declare variant' cannot be applied for function after first "
"usage; the original function might be used">, InGroup<SourceUsesOpenMP>;
Expand Down
16 changes: 0 additions & 16 deletions clang/include/clang/Basic/OpenMPKinds.def
Expand Up @@ -203,12 +203,6 @@
#ifndef OPENMP_DECLARE_VARIANT_CLAUSE
#define OPENMP_DECLARE_VARIANT_CLAUSE(Name)
#endif
#ifndef OPENMP_CONTEXT_SELECTOR_SET
#define OPENMP_CONTEXT_SELECTOR_SET(Name)
#endif
#ifndef OPENMP_CONTEXT_SELECTOR
#define OPENMP_CONTEXT_SELECTOR(Name)
#endif
#ifndef OPENMP_LASTPRIVATE_KIND
#define OPENMP_LASTPRIVATE_KIND(Name)
#endif
Expand All @@ -219,14 +213,6 @@
#define OPENMP_FLUSH_CLAUSE(Name)
#endif

// OpenMP context selector sets.
OPENMP_CONTEXT_SELECTOR_SET(implementation)
OPENMP_CONTEXT_SELECTOR_SET(device)

// OpenMP context selectors.
OPENMP_CONTEXT_SELECTOR(vendor)
OPENMP_CONTEXT_SELECTOR(kind)

// OpenMP clauses.
OPENMP_CLAUSE(allocator, OMPAllocatorClause)
OPENMP_CLAUSE(if, OMPIfClause)
Expand Down Expand Up @@ -1102,8 +1088,6 @@ OPENMP_FLUSH_CLAUSE(release)
#undef OPENMP_FLUSH_CLAUSE
#undef OPENMP_ORDER_KIND
#undef OPENMP_LASTPRIVATE_KIND
#undef OPENMP_CONTEXT_SELECTOR
#undef OPENMP_CONTEXT_SELECTOR_SET
#undef OPENMP_DECLARE_VARIANT_CLAUSE
#undef OPENMP_DEVICE_TYPE_KIND
#undef OPENMP_ALLOCATE_CLAUSE
Expand Down
39 changes: 0 additions & 39 deletions clang/include/clang/Basic/OpenMPKinds.h
Expand Up @@ -19,45 +19,6 @@

namespace clang {

/// OpenMP context selector sets.
enum OpenMPContextSelectorSetKind {
#define OPENMP_CONTEXT_SELECTOR_SET(Name) OMP_CTX_SET_##Name,
#include "clang/Basic/OpenMPKinds.def"
OMP_CTX_SET_unknown,
};

/// OpenMP context selectors.
enum OpenMPContextSelectorKind {
#define OPENMP_CONTEXT_SELECTOR(Name) OMP_CTX_##Name,
#include "clang/Basic/OpenMPKinds.def"
OMP_CTX_unknown,
};

OpenMPContextSelectorSetKind getOpenMPContextSelectorSet(llvm::StringRef Str);
llvm::StringRef
getOpenMPContextSelectorSetName(OpenMPContextSelectorSetKind Kind);
OpenMPContextSelectorKind getOpenMPContextSelector(llvm::StringRef Str);
llvm::StringRef getOpenMPContextSelectorName(OpenMPContextSelectorKind Kind);

/// Struct to store the context selectors info.
template <typename VectorType, typename ScoreT> struct OpenMPCtxSelectorData {
OpenMPContextSelectorSetKind CtxSet = OMP_CTX_SET_unknown;
OpenMPContextSelectorKind Ctx = OMP_CTX_unknown;
ScoreT Score;
VectorType Names;
explicit OpenMPCtxSelectorData() = default;
explicit OpenMPCtxSelectorData(OpenMPContextSelectorSetKind CtxSet,
OpenMPContextSelectorKind Ctx,
const ScoreT &Score, VectorType &&Names)
: CtxSet(CtxSet), Ctx(Ctx), Score(Score), Names(Names) {}
template <typename U>
explicit OpenMPCtxSelectorData(OpenMPContextSelectorSetKind CtxSet,
OpenMPContextSelectorKind Ctx,
const ScoreT &Score, const U &Names)
: CtxSet(CtxSet), Ctx(Ctx), Score(Score),
Names(Names.begin(), Names.end()) {}
};

/// OpenMP directives.
using OpenMPDirectiveKind = llvm::omp::Directive;

Expand Down

0 comments on commit 1228d42

Please sign in to comment.