[PATCH] D91944: OpenMP 5.0 metadirective

uweigand added a comment. In D91944#3009868 , @cchen wrote: > The SystemZ issue is due to the fact that we assumed that `device(cpu)` > should be evaluated to true and `device(gpu)` should be evaluated to false in > the test so the test should be fixed

[PATCH] D91944: OpenMP 5.0 metadirective

cchen added a comment. The SystemZ issue is due to the fact that we assumed that `device(cpu)` should be evaluated to true and `device(gpu)` should be evaluated to false in the test so the test should be fixed by specifying the triple.

[PATCH] D91944: OpenMP 5.0 metadirective

cchen added a comment. In D91944#3009003 , @uweigand wrote: > Looks like this was committed again, breaking the SystemZ build bots once > again: > https://lab.llvm.org/buildbot/#/builders/94/builds/5661 I'm going to fix this issue now. Repository:

[PATCH] D91944: OpenMP 5.0 metadirective

uweigand added a comment. Looks like this was committed again, breaking the SystemZ build bots once again: https://lab.llvm.org/buildbot/#/builders/94/builds/5661 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/

[PATCH] D91944: OpenMP 5.0 metadirective

davezarzycki added a comment. Also, when this lands again, please ensure that all the tests don't accidentally dump build artifacts into the source directory. Specifically, the following test was missing `-o -`: clang/test/OpenMP/metadirective_messages.cpp Repository: rG LLVM Github

[PATCH] D91944: OpenMP 5.0 metadirective

thakis added a comment. (Reverted for now since mac bots have been broken for 16 hours now.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 ___ cfe-commits

[PATCH] D91944: OpenMP 5.0 metadirective

phosek added a comment. We started seeing a test failure in `Clang :: OpenMP/metadirective_device_kind_codegen.c` on our macOS bot after this change landed (we aren't seeing it on other systems): Script: -- : 'RUN: at line 1'; /opt/s/w/ir/x/w/staging/llvm_build/bin/clang -cc1

[PATCH] D91944: OpenMP 5.0 metadirective

cchen added a comment. I reapplied the patch since I forgot to add author's name in the commit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 ___ cfe-commits

[PATCH] D91944: OpenMP 5.0 metadirective

This revision was automatically updated to reflect the committed changes. Closed by commit rGc7d7b98e5263: OpenMP 5.0 metadirective (authored by cchen). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 Files:

[PATCH] D91944: OpenMP 5.0 metadirective

cchen updated this revision to Diff 373312. cchen added a comment. Fix tests and coding style Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 Files: clang/include/clang-c/Index.h

[PATCH] D91944: OpenMP 5.0 metadirective

ABataev added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:5412 + + void setIfStmt(Stmt *stmt) { IfStmt = stmt; } + Stmt *getIfStmt() const { return IfStmt; } cchen wrote: > ABataev wrote: > > cchen wrote: > > > ABataev wrote: > > > > 1.

[PATCH] D91944: OpenMP 5.0 metadirective

cchen added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:5412 + + void setIfStmt(Stmt *stmt) { IfStmt = stmt; } + Stmt *getIfStmt() const { return IfStmt; } ABataev wrote: > cchen wrote: > > ABataev wrote: > > > 1. Make it private, please

[PATCH] D91944: OpenMP 5.0 metadirective

ABataev added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:5412 + + void setIfStmt(Stmt *stmt) { IfStmt = stmt; } + Stmt *getIfStmt() const { return IfStmt; } cchen wrote: > ABataev wrote: > > 1. Make it private, please > > 2. Var names

[PATCH] D91944: OpenMP 5.0 metadirective

cchen added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:5412 + + void setIfStmt(Stmt *stmt) { IfStmt = stmt; } + Stmt *getIfStmt() const { return IfStmt; } ABataev wrote: > 1. Make it private, please > 2. Var names must start from capital

[PATCH] D91944: OpenMP 5.0 metadirective

ABataev added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:5412 + + void setIfStmt(Stmt *stmt) { IfStmt = stmt; } + Stmt *getIfStmt() const { return IfStmt; } 1. Make it private, please 2. Var names must start from capital letter

[PATCH] D91944: OpenMP 5.0 metadirective

cchen updated this revision to Diff 373277. cchen added a comment. Fix tests for Windows Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 Files: clang/include/clang-c/Index.h

[PATCH] D91944: OpenMP 5.0 metadirective

cchen updated this revision to Diff 373139. cchen added a comment. Fix flang issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 Files: clang/include/clang-c/Index.h

[PATCH] D91944: OpenMP 5.0 metadirective

cchen updated this revision to Diff 373125. cchen added a comment. Rebase and fix warning Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 Files: clang/include/clang-c/Index.h

[PATCH] D91944: OpenMP 5.0 metadirective

cchen updated this revision to Diff 373109. cchen added a comment. Fix ast errors and add some tiny fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 Files: clang/include/clang-c/Index.h

[PATCH] D91944: OpenMP 5.0 metadirective

cchen updated this revision to Diff 372038. cchen added a comment. Remove debug info and spaces Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 Files: clang/include/clang-c/Index.h

[PATCH] D91944: OpenMP 5.0 metadirective

cchen updated this revision to Diff 372034. cchen added a comment. Remove redundant file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 Files: clang/include/clang-c/Index.h

[PATCH] D91944: OpenMP 5.0 metadirective

cchen updated this revision to Diff 372032. cchen added a comment. Herald added a reviewer: sscalpone. [OpenMP 5.0] metadirective This patch supports OpenMP 5.0 metadirective features. It is implemented keeping the OpenMP 5.1 features like dynamic user condition in mind. A new function,

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu added a comment. In D91944#2990234 , @cchen wrote: > @alokmishra.besu do you mind if I push the patch for solving those assertions > with rebase? The patch does not change the logic in your program and I've > made sure that the tests

[PATCH] D91944: OpenMP 5.0 metadirective

cchen added a comment. @alokmishra.besu do you mind if I push the patch for solving those assertions with rebase? The patch does not change the logic in your program and I've made sure that the tests could pass in debug mode. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D91944: OpenMP 5.0 metadirective

cchen added a comment. I'm guessing the tests were not pass on buildbot but passed on the author's side is due to the assertion was disabled on the author's side. Here is the patch for avoiding all the assertion errors and I'm able to get all the metadirective tests passed (and no regression

[PATCH] D91944: OpenMP 5.0 metadirective

protze.joachim added a comment. In D91944#2458154 , @jdoerfert wrote: > The problem is this patch can only resolve to a single directive during > parsing. Take > > template > void foo() { > #pragma omp metadirective when(user={condition(b)}) ...

[PATCH] D91944: OpenMP 5.0 metadirective

jdoerfert accepted this revision. jdoerfert added a comment. Asssuming this passes the tests I'll merge it as is. There are unresolved review requests, e.g., custom implementation instead of balanced parenthesis trackers, the template support, etc. but we decided we can make faster progress in

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu added a comment. In D91944#2540178 , @jdoerfert wrote: > There are no test cases anymore, as far as I can tell. Updated patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/

[PATCH] D91944: OpenMP 5.0 metadirective

jdoerfert added a comment. There are no test cases anymore, as far as I can tell. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 ___ cfe-commits mailing list

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu updated this revision to Diff 321202. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 Files: clang/include/clang-c/Index.h clang/include/clang/AST/RecursiveASTVisitor.h

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu updated this revision to Diff 321194. alokmishra.besu added a comment. I've rebase this patch with the latest git code. All test cases pass. I've also applied this patch to a new git clone. It applies and builds successfully. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu added a comment. In D91944#2505159 , @jdoerfert wrote: > In D91944#2485167 , @jdoerfert wrote: > >> Are the test all passing? > > Can we fix the failing tests so we can merge this? I will work on

[PATCH] D91944: OpenMP 5.0 metadirective

jdoerfert added a comment. In D91944#2485167 , @jdoerfert wrote: > Are the test all passing? Can we fix the failing tests so we can merge this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/

[PATCH] D91944: OpenMP 5.0 metadirective

jdoerfert added a comment. In D91944#2504924 , @protze.joachim wrote: > I found two issues with this patch regarding the `default` clause: > > - The spec does not require a default clause. I get `error: expected > expression` if I omit a default clause.

[PATCH] D91944: OpenMP 5.0 metadirective

protze.joachim added a comment. I found two issues with this patch regarding the `default` clause: - The spec does not require a default clause. I get `error: expected expression` if I omit a default clause. The error is gone if I add `default()`. - The spec does not allow an empty `default()`

[PATCH] D91944: OpenMP 5.0 metadirective

jdoerfert added a comment. Are the test all passing? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D91944: OpenMP 5.0 metadirective

cchen added a comment. @alokmishra.besu, I'm trying to play/learn with your patch so I downloaded the diff and apply it to the master branch, however, I'm getting error messages everywhere if `llvm::omp::OMPD_metadirective` appears: llvm-project/clang/include/clang/AST/StmtOpenMP.h:384:43:

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu updated this revision to Diff 314229. alokmishra.besu added a comment. Rebased with current branch to avoid patch failure Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 Files:

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu added a comment. Emitting error instead of instantiation in clang/lib/Sema/TreeTransform.h. Added a TODO for future. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu updated this revision to Diff 314224. alokmishra.besu marked an inline comment as done. alokmishra.besu added a comment. All clang test and clang-tidy pass. Ran clang-format on all code, except clang/include/clang/Serialization/ASTBitCodes.h which was formatting unrelated code

[PATCH] D91944: OpenMP 5.0 metadirective

ABataev added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:8383-8388 + DeclarationNameInfo DirName; + getDerived().getSema().StartOpenMPDSABlock(OMPD_metadirective, DirName, + nullptr, D->getBeginLoc()); + StmtResult

[PATCH] D91944: OpenMP 5.0 metadirective

jdoerfert added a comment. (Please make sure all clang and clang-tidy tests pass properly so we can merge the patch) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944

[PATCH] D91944: OpenMP 5.0 metadirective

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. As discussed in the OpenMP in LLVM call on December 9th, we should go ahead with this patch and add support for the missing features, e.g., dependent selectors, afterwards. LGTM

[PATCH] D91944: OpenMP 5.0 metadirective

jdoerfert added a comment. In D91944#2457744 , @dreachem wrote: > In D91944#2414364 , @jdoerfert wrote: > >> This looks close to an OpenMP 5.0 implementation. I left comments inlined. >> >> We need tests that show

[PATCH] D91944: OpenMP 5.0 metadirective

dreachem added a comment. In D91944#2414364 , @jdoerfert wrote: > This looks close to an OpenMP 5.0 implementation. I left comments inlined. > > We need tests that show how non-selected alternatives *do not* impact the > program. As an example, a

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu added inline comments. Comment at: clang/include/clang/Serialization/ASTBitCodes.h:1952 - STMT_MS_DEPENDENT_EXISTS, // MSDependentExistsStmt - EXPR_LAMBDA,// LambdaExpr STMT_COROUTINE_BODY, jdoerfert wrote: >

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu updated this revision to Diff 309382. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 Files: clang/include/clang-c/Index.h clang/include/clang/AST/RecursiveASTVisitor.h

[PATCH] D91944: OpenMP 5.0 metadirective

jdoerfert added inline comments. Comment at: clang/include/clang/Serialization/ASTBitCodes.h:1952 - STMT_MS_DEPENDENT_EXISTS, // MSDependentExistsStmt - EXPR_LAMBDA,// LambdaExpr STMT_COROUTINE_BODY, alokmishra.besu wrote: >

[PATCH] D91944: OpenMP 5.0 metadirective

ABataev added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:373 +/// +class OMPMetaDirective final : public OMPExecutableDirective { + friend class ASTStmtReader; alokmishra.besu wrote: > ABataev wrote: > > I think, metadirective should be a

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:373 +/// +class OMPMetaDirective final : public OMPExecutableDirective { + friend class ASTStmtReader; ABataev wrote: > I think, metadirective should be a kind of a

[PATCH] D91944: OpenMP 5.0 metadirective

ABataev added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:373 +/// +class OMPMetaDirective final : public OMPExecutableDirective { + friend class ASTStmtReader; I think, metadirective should be a kind of a container for different

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu updated this revision to Diff 309268. alokmishra.besu added a comment. Updated version of metadirective supporting OpenMP 5.0 features Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 Files:

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu added a comment. I have replied to the comments and will update the code accordingly. Some of the codes are intentionally left to be update in 5.1 implementation. Will add TODO comments there. I will revisit the implementation of getBestWhenMatchForContext and also add more

[PATCH] D91944: OpenMP 5.0 metadirective

jdoerfert added a comment. This looks close to an OpenMP 5.0 implementation. I left comments inlined. We need tests that show how non-selected alternatives *do not* impact the program. As an example, a template instantiation inside of a non-selected alternative is not actually performed. We

[PATCH] D91944: OpenMP 5.0 metadirective

alokmishra.besu created this revision. alokmishra.besu added a project: OpenMP. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, martong, arphaman, guansong, hiraditya, yaxunl. Herald added projects: clang, LLVM. alokmishra.besu requested review of this revision. Herald added a