[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

jdoerfert added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:7641 + /// (which accepts anything) and (later) extensions. + StringRef RawString; }; mikerice wrote: > This field doesn't seem to have serialization code. Is that expected

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

mikerice added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:7641 + /// (which accepts anything) and (later) extensions. + StringRef RawString; }; This field doesn't seem to have serialization code. Is that expected or an oversight?

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

thakis added a comment. That did the trick, thanks! The bot was broken for > 5h and 43 builds. We got lucky that nothing else broke in the meantime, but red bots make it more difficult to analyze new regressions that appear when the bots are already red. Next time, please revert while you

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

jdoerfert added a comment. In D83281#2183100 , @thakis wrote: > Looks like it's still failing with that change: > http://45.33.8.238/win/20885/step_7.txt dso_local is *before* the void,.. 2 min Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

thakis added a comment. Looks like it's still failing with that change: http://45.33.8.238/win/20885/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83281/new/ https://reviews.llvm.org/D83281

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

jdoerfert added a comment. In D83281#2182507 , @thakis wrote: > This breaks check-clang on Windows: http://45.33.8.238/win/20850/step_7.txt > > Please take a look, and revert while you investigate if it takes a while to > fix. Should be fixed by

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

thakis added a comment. This breaks check-clang on Windows: http://45.33.8.238/win/20850/step_7.txt Please take a look, and revert while you investigate if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83281/new/

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGee05167cc42b: [OpenMP] Allow traits for the OpenMP context selector `isa` (authored by jdoerfert). Changed prior to commit:

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG with a nit. Comment at: clang/lib/AST/OpenMPClause.cpp:2304-2306 + auto it = FeatureMap.find(RawString); + if (it != FeatureMap.end()) +return it->second;

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

jdoerfert added a comment. ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83281/new/ https://reviews.llvm.org/D83281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

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

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

jdoerfert updated this revision to Diff 280151. jdoerfert marked an inline comment as done. jdoerfert added a comment. Add virtual Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83281/new/ https://reviews.llvm.org/D83281 Files:

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

ABataev added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:7663 + const FunctionDecl *CurrentFunctionDecl); + ~TargetOMPContext() = default; + `virtual` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

jdoerfert updated this revision to Diff 280148. jdoerfert marked 6 inline comments as done. jdoerfert added a comment. Addressed @ABataev comments, thx! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83281/new/ https://reviews.llvm.org/D83281

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

ABataev added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:7599 + /// (which accept anything) and (later) extensions. + StringRef RawString{}; }; No need for the default initializer here Comment at:

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

jdoerfert added a comment. ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83281/new/ https://reviews.llvm.org/D83281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

jdoerfert updated this revision to Diff 278175. jdoerfert added a comment. Add warning and negative test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83281/new/ https://reviews.llvm.org/D83281 Files: clang/include/clang/AST/OpenMPClause.h

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

JonChesterfield added a comment. Agreed on tests. I like the mechanism - passing a string through to the backend as a way to dispatch between isa properties looks cleanly extensible. We probably do want to emit a warning when the backend claims it doesn't know anything about said string as

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

jdoerfert created this revision. jdoerfert added reviewers: jhuber6, fghanim, JonChesterfield, grokos, ggeorgakoudis, ABataev. Herald added subscribers: llvm-commits, sstefan1, guansong, bollu, hiraditya, yaxunl. Herald added projects: clang, LLVM. NOTE: The changes are fairly mechanical