[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

This revision was automatically updated to reflect the committed changes. Closed by commit rG095cecbe0ded: [OpenMP] `omp begin/end declare variant` - part 1, parsing (authored by jdoerfert). Herald added a subscriber: hiraditya. Changed prior to commit: https://reviews.llvm.org/D74941?vs=246630

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74941/new/ https://reviews.llvm.org/D74941

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert updated this revision to Diff 246630. jdoerfert marked 5 inline comments as done. jdoerfert added a comment. Addressed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74941/new/ https://reviews.llvm.org/D74941 Files: clang/inclu

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

aaron.ballman added inline comments. Comment at: clang/include/clang/Parse/Parser.h:2959 + /// if there was an error. + bool ParseOMPDeclareVariantMatchClause(SourceLocation Loc, OMPTraitInfo &TI); + This file is inconsistent currently, but it looks like `parse

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

JonChesterfield added a subscriber: gregrodgers. JonChesterfield added a comment. Thanks! Splitting this out of D71179 , which I think ultimately reached consensus, makes the diff much easier to read. Subscribing Greg, as I think this is a path to removing a lot

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert updated this revision to Diff 246094. jdoerfert added a comment. minor adjustment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74941/new/ https://reviews.llvm.org/D74941 Files: clang/include/clang/AST/OpenMPClause.h clang/include/cl

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert updated this revision to Diff 246035. jdoerfert added a comment. Revert test check lines to the parser part only Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74941/new/ https://reviews.llvm.org/D74941 Files: clang/include/clang/AST/Op

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert updated this revision to Diff 246032. jdoerfert marked an inline comment as done. jdoerfert added a comment. Herald added a subscriber: jfb. Add tests and address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74941/new/ https://r

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: clang/test/OpenMP/begin-declare-variant_elided_range_withouth_end.c:5 +// TODO: Improve the error message +#pragma omp begin declare variant match(device={kind(cpu)}) // expected-error {{un

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert marked 6 inline comments as done. jdoerfert added a comment. In D74941#1887367 , @fpetrogalli wrote: > 1. I am not familiar with `OpenMP technical report 8 (TR8)`. Could you please > provide a link in the description? Same for PR42061, PR42798,

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

ABataev added a comment. In D74941#1887381 , @kkwli0 wrote: > Since it is a TR8 feature, should we have this guarded by `-fopenmp-version=`? Not this one. It is required to fix the issues with math functions and should work for all versions of OpenMP. O

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

kkwli0 added a comment. Since it is a TR8 feature, should we have this guarded by `-fopenmp-version=`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74941/new/ https://reviews.llvm.org/D74941 ___ cfe-c

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

fpetrogalli added a comment. Hi @jdoerfert , thank you for working on this. I have a couple of high level comments. 1. I am not familiar with `OpenMP technical report 8 (TR8)`. Could you please provide a link in the description? Same for PR42061, PR42798, PR42799. It would be useful to get an

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

kkwli0 added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1251 "expected '#pragma omp end declare target'">; +def err_expected_end_declare_target_or_variant : Error< + "expected '#pragma omp end declare %select{target|variant}0'">; --

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert marked 2 inline comments as done. jdoerfert added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:1357-1359 + // Skip last tokens. + while (Tok.isNot(tok::annot_pragma_openmp_end)) +ConsumeAnyToken(); ABataev wrote: > Better to emit a

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert added a comment. In D74941#1886403 , @kiranchandramohan wrote: > Will tests come in a later patch? I have tests, seems I forgot to add them. Will fix later today with the other things. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

ABataev added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:1357-1359 + // Skip last tokens. + while (Tok.isNot(tok::annot_pragma_openmp_end)) +ConsumeAnyToken(); Better to emit a warning here about extra tokens at the end of the directive

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

kiranchandramohan added a comment. Will tests come in a later patch? Comment at: clang/include/clang/AST/OpenMPClause.h:6700 /// independent of clang. Thus, expressions and conditions are evaluated in - /// this method. + /// this method. If \p DeviceSetOnly is set only th

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert created this revision. jdoerfert added reviewers: kiranchandramohan, ABataev, RaviNarayanaswamy, gtbercea, grokos, sdmitriev, JonChesterfield, hfinkel, fghanim. Herald added subscribers: llvm-commits, guansong, bollu, jholewinski. Herald added projects: clang, LLVM. This is the first pa