[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:16393-16416 +static DefaultMapImplicitBehavior +getImplicitBehaviorFromModifier(OpenMPDefaultmapClauseModifier M) { + switch (M) { + case OMPC_DEFAULTMAP_MODIFIER_allo

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

cchen marked 2 inline comments as done. cchen added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:2339-2345 +unsigned Modifier = getOpenMPSimpleClauseType( +Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok)); +// Set defaultmap modifier to unknown

[PATCH] D70142: [OpenMP5.0] Fix a parsing issue for the extended defaultmap

cchen created this revision. cchen added a reviewer: ABataev. Herald added subscribers: cfe-commits, guansong. Herald added a reviewer: jdoerfert. Herald added a project: clang. cchen retitled this revision from "[OpenMP5.0] Fix a parsing issue for the extended defaultmap This patch is for fixing

[PATCH] D70142: [OpenMP5.0] Fix a parsing issue for the extended defaultmap

cchen added a comment. Not sure how to test this change since the issue only happens after I add more implicit-behaviors for extended defaultmap. Now `ActOnOpenMPDefaultmapClause` is doing `M != OMPC_DEFAULTMAP_MODIFIER_tofrom`, so even the modifier is set to be "scalar", the code is still rig

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/test/OpenMP/target_defaultmap_messages.cpp:49 + + #pragma omp target defaultmap(tofrom:scalar) defaultmap(to:scalar) // expected-error {{at most one defaultmap clause for each variable-catego

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/test/OpenMP/target_defaultmap_messages.cpp:49 + + #pragma omp target defaultmap(tofrom:scalar) defaultmap(to:scalar) // expected-error {{at most one defaultmap clause for each variable-catego

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

cchen marked 2 inline comments as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:2953-2954 +(!(isOpenMPTargetExecutionDirective(DKind) && Res && + (*Res == OMPDeclareTargetDeclAttr::MT_Link || +*Res == OMPDec

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/test/OpenMP/target_defaultmap_codegen.cpp:1538-1546 +void declare_target_link(float A) +{ + // CK26: [[AADDR:%.+]] = alloca float* + // CK26: store float* [[A:%.+]], float** [[AADDR]] + // C

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

cchen added a comment. Alexey, thanks so much for your help, you really make me learn so much about Clang and coding in general. Can you land this patch for me since I'm still new to llvm/clang and haven't have the commit access. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D69316: [OpenMP 5.0] target update list items need not be contiguous (Sema)

cchen updated this revision to Diff 229922. cchen added a comment. Add and fix test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69316/new/ https://reviews.llvm.org/D69316 Files: clang/lib/Sema/SemaOpenMP.cpp clang/test/OpenMP/target_update_

[PATCH] D70726: [OpenMP50] Add parallel master construct

cchen created this revision. cchen added a reviewer: ABataev. Herald added subscribers: cfe-commits, jfb, arphaman, guansong, jholewinski. Herald added a reviewer: jdoerfert. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D70726 Files: clang/inclu

[PATCH] D70726: [OpenMP50] Add parallel master construct

cchen marked 2 inline comments as done. cchen added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2933 +OMPPrivateScope PrivateScope(CGF); +bool Copyins = CGF.EmitOMPCopyinClause(S); +(void)CGF.EmitOMPFirstprivateClause(S, PrivateScope);

[PATCH] D70726: [OpenMP50] Add parallel master construct

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/test/OpenMP/nesting_of_regions.cpp:3024 + { +#pragma omp master // OK, though second 'master' is redundant +{ ABataev wrote: > Is this allowed by the standard? I didn't se

[PATCH] D70726: [OpenMP50] Add parallel master construct

cchen added a comment. @ABataev, thanks for your time and patient for the review! Could you land this patch for me when you have free time? Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70726/new/ https://reviews.llvm.org/D70726

[PATCH] D70726: [OpenMP50] Add parallel master construct

cchen added a comment. How to see the test result on Windows? I've no windows machine to test the compiler. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70726/new/ https://reviews.llvm.org/D70726 ___

[PATCH] D70726: [OpenMP50] Add parallel master construct

cchen added a comment. I've found it: http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/1042 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70726/new/ https://reviews.llvm.org/D70726 ___

[PATCH] D70726: [OpenMP50] Add parallel master construct

cchen added a comment. Can you commit it again for me? thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70726/new/ https://reviews.llvm.org/D70726 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

cchen created this revision. cchen added a reviewer: ABataev. Herald added subscribers: cfe-commits, jfb, guansong. Herald added a reviewer: jdoerfert. Herald added a project: clang. cchen edited the summary of this revision. cchen added a project: OpenMP. cchen added subscribers: dreachem, sandova

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

cchen added a comment. Oops, accidentally remove my own comment. I'm not sure why `iarr[foo(), foo(), 0]` violate the rule since it will be evaluated as `iarr[0]`, and the counter `foo()` modified is also in the same location for both left hand side and right hand side. Repository: rG LLVM

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

cchen added a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71225/new/ https://reviews.llvm.org/D71225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mail

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

cchen added a comment. @jdoerfert , thanks for your explanation, it's really clear and helpful. I'll add a warning message in another patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71225/new/ https://reviews.llvm.org/D71225 __

[PATCH] D71294: [OpenMP] Emit warning message about undefined number of evaluation for atomic update/capture

cchen created this revision. cchen added a reviewer: ABataev. Herald added subscribers: cfe-commits, jfb, guansong. Herald added a reviewer: jdoerfert. Herald added a project: clang. The total number of evaluation of X is undefined for atomic construct, therefore, we emit warning message for them.

[PATCH] D71294: [OpenMP] Emit warning message about undefined number of evaluation for atomic update/capture

cchen planned changes to this revision. cchen added a comment. Don't know there is an analysis phase in Clang, I'll investigate later, thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71294/new/ https://reviews.llvm.org/D71294 _

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen created this revision. cchen added a reviewer: ABataev. Herald added subscribers: cfe-commits, guansong. Herald added a reviewer: jdoerfert. Herald added a project: clang. For this example: int a[100]; int f2 (int i, int k) { for (int j = 16; j < 64; j++) { a[

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen added a comment. In D71475#1783834 , @jdoerfert wrote: > Your commit message example lacks the #pragma. > > What if you add k to the list of explicit firstprivate? (I mean, you can try > it in C first). > > And how do I reproduce the crash? I tried:

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen updated this revision to Diff 233866. cchen added a comment. Add linear step var into Implicitfirstprivate Doing this still fail the assertion since we still don't have the variable inside CapturedStmt. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen updated this revision to Diff 233869. cchen added a comment. Remove debug code and some redundancy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71475/new/ https://reviews.llvm.org/D71475 Files: clang/lib/Sema/SemaOpenMP.cpp clang/test/O

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen marked an inline comment as done. cchen added a comment. In D71475#1784284 , @jdoerfert wrote: > What is the output when you run the example with `k` in `lastprivate` or > `reduction`? I actually got the same result (return value) changing from fi

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen added a comment. > It will still crash for something like `target simd`, need to move the code > around a little bit, move it out of `if`. I tried making `target simd ` crash but it seems work (target_simd_ast_print.cpp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen updated this revision to Diff 234157. cchen added a comment. Apply @ABataev's patch, add some code to support the patch and some tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71475/new/ https://reviews.llvm.org/D71475 Files: clang/li

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128 + (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate || + Data.Attributes == OMPC_linear)) ||

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen added a comment. In D71475#1787914 , @ABataev wrote: > Here is more proper fix. We don't need to capture just `k` here, instead, we > need to capture the whole expression. > Linear clause has a little bit different processing rather than all other

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen added a comment. In D71475#1788187 , @ABataev wrote: > The fixed patch. Several codegen tests require some adjustment. @Abataev, thanks for the reply. However, for adjusting the codegen, I found it really hard to update the tests by reading the di

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15554 +LocatorChecker Checker; +if (Checker.Visit(OrigExpr)) { + llvm::copy(Checker.getComponents(), ABataev wrote: > General q

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15554 +LocatorChecker Checker; +if (Checker.Visit(OrigExpr)) { + llvm::copy(Checker.getComponents(), ABataev wrote: > cchen wro

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen updated this revision to Diff 243950. cchen added a comment. 1. Refactor based on feedback and use the visitor class to integrate the analysis in checkMapClauseExpressionBase. 2. Handle more lvalue cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added a comment. > Quoted Text Comment at: clang/lib/Sema/SemaOpenMP.cpp:15554 +LocatorChecker Checker; +if (Checker.Visit(OrigExpr)) { + llvm::copy(Checker.getComponents(), ABataev wrote:

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15293 + // TypeDecorCnt for B: 0 + if (Depth <= TypeDecorCnt) { +RelevantExpr = cast(DRE); ABataev wrote: > The check is really bad.

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked 2 inline comments as done. cchen added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7709-7712 isa(Next->getAssociatedExpression()) || isa(Next->getAssociatedExpression()) || -isa(Next->getAssocia

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15442-15443 + CurComponents.emplace_back(CurE, nullptr); +} else if (auto *CurE = dyn_cast(E)) { + E = CurE->getLHS()->IgnoreParenImpCasts(); } else {

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15442-15443 + CurComponents.emplace_back(CurE, nullptr); +} else if (auto *CurE = dyn_cast(E)) { + E = CurE->getLHS()->IgnoreParenImpCasts(); } else {

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15442-15443 + CurComponents.emplace_back(CurE, nullptr); +} else if (auto *CurE = dyn_cast(E)) { + E = CurE->getLHS()->IgnoreParenImpCasts(); } else {

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15442-15443 + CurComponents.emplace_back(CurE, nullptr); +} else if (auto *CurE = dyn_cast(E)) { + E = CurE->getLHS()->IgnoreParenImpCasts(); } else {

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen updated this revision to Diff 240029. cchen added a comment. Add a checker for locator (test is not complete yet). The logic of finding basic decl in LocatorChecker: - `int *B; omp target map(*B)` - B - pointer count (* in type): 1 - deref count: 1 - B is basic decl since poi

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked 2 inline comments as done. cchen added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7829 if (EncounteredME) { - const auto *FD = dyn_cast(EncounteredME->getMemberDecl()); - unsigned FieldIndex = FD->getFieldIndex(); -

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added a comment. In D72811#1837392 , @jdoerfert wrote: > Thanks for working on this! While you are at it, `*this` is probably one of > the most important ones to test and support. Can anyone tell me how to g

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15235 + bool VisitUnaryOperator(UnaryOperator *UO) { +DerefCnt++; +CurComponents.emplace_back(UO, nullptr); cchen wrote: > ABataev wrote: > > N

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15235 + bool VisitUnaryOperator(UnaryOperator *UO) { +DerefCnt++; +CurComponents.emplace_back(UO, nullptr); ABataev wrote: > cchen wrote: > > c

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added a comment. In D72811#1839561 , @ABataev wrote: > In D72811#1839538 , @cchen wrote: > > > In D72811#1837392 , @jdoerfert >

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen updated this revision to Diff 241291. cchen added a comment. Fix based on feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72811/new/ https://reviews.llvm.org/D72811 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen updated this revision to Diff 241293. cchen added a comment. Update due to negligence Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72811/new/ https://reviews.llvm.org/D72811 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/

[PATCH] D100874: [OpenMP] Use irbuilder as default for masked and master construct

cchen updated this revision to Diff 340641. cchen added a comment. This revision is now accepted and ready to land. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100874/new/ https://reviews.llvm.org/D100874 Files: clang/lib/CodeGen/CGStmt

[PATCH] D115901: [OpenMP][NFC] update status for 5.1 'fail' atomic extension

cchen accepted this revision. cchen added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115901/new/ https://reviews.llvm.org/D115901 ___ cfe

[PATCH] D99622: [OpenMP51] Accept `primary` as proc bind affinity policy in Clang

cchen created this revision. cchen added a reviewer: ABataev. Herald added subscribers: guansong, yaxunl. cchen requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: llvm-commits, cfe-commits, sstefan1. Herald added projects: clang, LLVM. Repository: rG

[PATCH] D99622: [OpenMP51] Accept `primary` as proc bind affinity policy in Clang

cchen updated this revision to Diff 334482. cchen added a comment. Add some codegen tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99622/new/ https://reviews.llvm.org/D99622 Files: clang/lib/Sema/SemaOpenMP.cpp clang/test/OpenMP/distribut

[PATCH] D99622: [OpenMP51] Accept `primary` as proc bind affinity policy in Clang

cchen added inline comments. Comment at: clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp:110-147 +// Test target codegen - host bc file has to be created first. +// RUN: %clang_cc1 -DCK2 -verify -fopenmp -fopenmp-version=45 -fopenmp-cuda-mode -x c++ -triple powerp

[PATCH] D99622: [OpenMP51] Accept `primary` as proc bind affinity policy in Clang

cchen updated this revision to Diff 334720. cchen added a comment. Update codegen test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99622/new/ https://reviews.llvm.org/D99622 Files: clang/lib/Sema/SemaOpenMP.cpp clang/test/OpenMP/parallel_ast

[PATCH] D99622: [OpenMP51] Accept `primary` as proc bind affinity policy in Clang

cchen updated this revision to Diff 334844. cchen added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99622/new/ https://reviews.llvm.org/D99622 Files: clang/lib/Sema/SemaOpenMP.cpp clang/test/OpenMP/parallel_ast_print.cpp

[PATCH] D99622: [OpenMP51] Accept `primary` as proc bind affinity policy in Clang

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcba422264c7f: [OpenMP51] Accept `primary` as proc bind affinity policy in Clang (authored by cchen). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D99995: [OpenMP51] Initial support for masked directive and filter clause

cchen added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:2814 } if (CKind == OMPC_ordered && PP.LookAhead(/*N=*/0).isNot(tok::l_paren)) ABataev wrote: > This can be simplified like this: > ``` > if (!FirstClause) { > Diag(Tok

[PATCH] D99995: [OpenMP51] Initial support for masked directive and filter clause

cchen updated this revision to Diff 336537. cchen added a comment. Fix lit-test failure for dispatch_ast_print and rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5/new/ https://reviews.llvm.org/D5 Files: clang/include/clang-c/Index

[PATCH] D99995: [OpenMP51] Initial support for masked directive and filter clause

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1a43fd27691c: [OpenMP51] Initial support for masked directive and filter clause (authored by cchen). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D100514: [OpenMP] Added codegen for masked directive

cchen created this revision. cchen added a reviewer: ABataev. Herald added subscribers: guansong, hiraditya, yaxunl. cchen requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: llvm-commits, cfe-commits, sstefan1. Herald added projects: clang, LLVM. Repos

[PATCH] D100514: [OpenMP] Added codegen for masked directive

cchen updated this revision to Diff 337797. cchen added a comment. Update test and fix code format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100514/new/ https://reviews.llvm.org/D100514 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp clang/li

[PATCH] D100514: [OpenMP] Added codegen for masked directive

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe0c2125d1d1e: [OpenMP] Added codegen for masked directive (authored by cchen). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D100514: [OpenMP] Added codegen for masked directive

cchen added a comment. In D100514#2693600 , @jdoerfert wrote: > Any reason we should not unconditionally use the OMPIRBuilder impl? (btw, > many thanks for providing one!) > We have an OMPIRBuilder always around in clang's codegen, so there is little >

[PATCH] D100514: [OpenMP] Added codegen for masked directive

cchen added a comment. In D100514#2693601 , @jdoerfert wrote: > Also, don't forget to mark it as done in > https://clang.llvm.org/docs/OpenMPSupport.html :) I'll mark it as done after combined constructs are also done. Thanks! Repository: rG LLVM Gi

[PATCH] D100514: [OpenMP] Added codegen for masked directive

cchen added a comment. > Initially we did not have an OMPIRBuilder object unconditionally, now we > have. Let's move over everything that is ready. So master and critical should > be good to go as well I suppose. While using OMPIRBuilder as default, do we want to just remove codegen in Clang o

[PATCH] D100874: [OpenMP] Use irbuilder as default for masked and master construct

cchen created this revision. cchen added a reviewer: jdoerfert. Herald added subscribers: guansong, yaxunl. cchen requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D1008

[PATCH] D100514: [OpenMP] Added codegen for masked directive

cchen added a comment. Use irbuilder for masked and master construct: https://reviews.llvm.org/D100874. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100514/new/ https://reviews.llvm.org/D100514 ___ cfe-

[PATCH] D100874: [OpenMP] Use irbuilder as default for masked and master construct

cchen planned changes to this revision. cchen added a comment. This change seems to expose bugs in IRBuilder. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100874/new/ https://reviews.llvm.org/D100874 __

[PATCH] D100874: [OpenMP] Use irbuilder as default for masked and master construct

cchen added a comment. I mark this patch as "plan changed" since the assert message indicates that something wrong in IRBuilder or Codegen, however, main branch also have the same issue so I think this patch does not trigger the issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D100874: [OpenMP] Use irbuilder as default for masked and master construct

cchen added a comment. > what assertion? $ ":" "RUN: at line 1" $ "/mnt/disks/ssd0/agent/llvm-project/build/./bin/clang" "-fopenmp" "-pthread" "-fno-experimental-isel" "-I" "/mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test" "-I" "/mnt/disks/ssd0/agent/llvm-project/build/projects/op

[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: er

[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 for

[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 h

[PATCH] D109635: [WIP][OpenMP] Support construct trait set for Clang

cchen created this revision. cchen added a reviewer: jdoerfert. Herald added subscribers: guansong, yaxunl. cchen requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. This patch supports construct trait set selector by using the existed

[PATCH] D109635: [WIP][OpenMP] Support construct trait set for Clang

cchen added inline comments. Comment at: clang/lib/AST/OpenMPClause.cpp:2346 - VMI.ConstructTraits.push_back(Selector.Properties.front().Kind); + // VMI.ConstructTraits.push_back(Selector.Properties.front().Kind); } I'm now removing this line sin

[PATCH] D109635: [WIP][OpenMP] Support construct trait set for Clang

cchen added inline comments. Comment at: clang/test/OpenMP/declare_variant_construct_codegen_1.c:23 +#define N 100 + +void p_vxv(int *v1, int *v2, int *v3, int n); This test is from sollve test suite: https://github.com/SOLLVE/sollve_vv/blob/master/tests/5.0/dec

[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, getBestW

[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 clang/include/clang/AST/Recu

[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 clang/include/clang/A

[PATCH] D109635: [WIP][OpenMP] Support construct trait set for Clang

cchen added a comment. > One thing I wanted to do though is keep track of the constructs in the > OpenMPIRBuilder instead. So move the > > /// Vector of declare variant construct traits. > SmallVector ConstructTraits; > > into OpenMPIRBuilder and add/delete the things there. It's not a concep

[PATCH] D109635: [WIP][OpenMP] Support construct trait set for Clang

cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:737 + } + void eraseConstructTrait() { ConstructTraits.clear(); } + cchen wrote: > jdoerfert wrote: > > Modification was made to make it clear what happens but now I think this is > > not n

[PATCH] D109635: [WIP][OpenMP] Support construct trait set for Clang

cchen updated this revision to Diff 372347. cchen added a comment. Rebase and fix based on suggestions Haven't ConstructTrait to IRBuilder Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109635/new/ https://reviews.llvm.org/D109635 Files: clang/i

[PATCH] D109635: [WIP][OpenMP] Support construct trait set for Clang

cchen updated this revision to Diff 372529. cchen added a comment. Fix based on feedback (wait for comment about moving ConstructTrait to IRBuilder) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109635/new/ https://reviews.llvm.org/D109635 Files:

[PATCH] D109635: [WIP][OpenMP] Support construct trait set for Clang

cchen updated this revision to Diff 372531. cchen added a comment. Remove braces Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109635/new/ https://reviews.llvm.org/D109635 Files: clang/include/clang/AST/OpenMPClause.h clang/lib/AST/OpenMPClaus

[PATCH] D109635: [WIP][OpenMP] Support construct trait set for Clang

cchen updated this revision to Diff 372584. cchen added a comment. Fix based on suggestions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109635/new/ https://reviews.llvm.org/D109635 Files: clang/include/clang/AST/OpenMPClause.h clang/lib/AST/

[PATCH] D109635: [WIP][OpenMP] Support construct trait set for Clang

cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:746 +} + } + } jdoerfert wrote: > jdoerfert wrote: > > So, SmalVector has a pop_back_val which we could use to verify the > > properties are the ones we expect. However, we woul

[PATCH] D109635: [WIP][OpenMP] Support construct trait set for Clang

cchen updated this revision to Diff 372843. cchen added a comment. Add tests and fix coding style Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109635/new/ https://reviews.llvm.org/D109635 Files: clang/include/clang/AST/OpenMPClause.h clang/li

<    1   2   3