Adds initial parsing and sema for the 'append_args' clause.
Note that an AST clause is not created as it instead adds its values
to the OMPDeclareVariantAttr.
Paths
| Differential D111854
[OPENMP51]Initial parsing/sema for append_args clause for 'declare variant' ClosedPublic Authored by mikerice on Oct 14 2021, 4:58 PM.
Details Summary Adds initial parsing and sema for the 'append_args' clause. Note that an AST clause is not created as it instead adds its values
Diff Detail Event TimelineComment Actions I'm not super familiar with OpenMP code, but are changes needed for AST serialization as well (so that PCH, modules, etc continue to work properly)?
Comment Actions Thanks for the review! As far as serialization goes, yes we need it, and the OpenMP tests test this is the ast print and codegen tests. This patch doesn't need anything special since it is just adding another field to the attribute which is serialized correctly with tablegen code afaik.
Comment Actions
Hmmm, I took a look through the serialization code that includes OMP.inc and I don't see anything to suggest that this addition causes a change to the AST version. So the situation I'm worried about is where new Clang generates a PCH file with this feature and then hands it to an older Clang without this feature. If you think we have sufficient test coverage for this sort of thing, then okay. But if you think there's not sufficient test coverage, this might be worth a follow-up patch to improve test coverage in general. The rest of my comments are mostly nits because I'm not really an expert in OpenMP. :-)
mikerice added inline comments.
mikerice marked an inline comment as done. Comment ActionsAdded tests for virtual and pure virtual functions.
This revision is now accepted and ready to land.Oct 25 2021, 6:29 AM This revision was landed with ongoing or failed builds.Oct 25 2021, 9:43 AM Closed by commit rGd8699391a431: [OPENMP51]Initial parsing/sema for append_args clause for 'declare variant' (authored by mikerice). · Explain Why This revision was automatically updated to reflect the committed changes. Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 25 2021, 9:43 AM
Revision Contents
Diff 380795 clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/AttrImpl.cpp
clang/lib/Basic/OpenMPKinds.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/OpenMP/declare_variant_clauses_ast_print.cpp
clang/test/OpenMP/declare_variant_clauses_messages.cpp
flang/lib/Semantics/check-omp-structure.cpp
llvm/include/llvm/Frontend/OpenMP/OMP.td
|
I'm hoping this diagnostic makes more sense in situ because I'm not certain what it's telling me. append-op is not a term of art we've used elsewhere, is there a better term we could use or will this be familiar to OpenMP users?