[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-22 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-22 Thread Sunil K via Phabricator via cfe-commits
koops added inline comments. Comment at: clang/lib/Basic/CMakeLists.txt:4 TargetParser + FrontendOpenMP ) koops wrote: > ABataev wrote: > > What requires this new dependency? > When cmake uses -DBUILD_SHARED_LIBS=1 shared libraries are built instead of >

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-22 Thread Sunil K via Phabricator via cfe-commits
koops requested review of this revision. koops added inline comments. Comment at: clang/lib/Basic/CMakeLists.txt:4 TargetParser + FrontendOpenMP ) ABataev wrote: > What requires this new dependency? When cmake uses -DBUILD_SHARED_LIBS=1 shared libraries ar

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-22 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/lib/Basic/CMakeLists.txt:4 TargetParser + FrontendOpenMP ) What requires this new dependency? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 __

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-22 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 558146. koops added a comment. Adding libFrontendOpenMP.so as a dependent library when building libclangBasic.so. Shared libraries were not built and tested by default, hence build and failed when checking on ppc64le. CHANGES SINCE LAST ACTION https://revi

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. FWIW, this appears to have broken some build bots: https://lab.llvm.org/buildbot/#/builders/121/builds/36635 -- can you revert and investigate (or fix-forward quickly)? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D1

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-17 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 558120. koops added a comment. Moving the declaration and definition of checkFailClauseParameter to include/clang/Basic/OpenMPKinds.h & lib/Basic/OpenMPKinds.cpp respectively. 2 other minor changes suggested by Alexey. CHANGES SINCE LAST ACTION https://revi

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-16 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:2516 +extern bool checkFailParameter(OpenMPClauseKind FailParameter); +/// This represents 'fail' clause in the '#pragma omp atomic' No way for extern functions. Declare in includ

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-16 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 558112. koops added a comment. 1. Added a check for the fail parameter before the instance of OMPFailClause is created in ActOnOpenMPFailClause. An error in fail parameter like `#pragma omp atomic compare fail(capture)` was creating a wrong instance of OMPFail

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-13 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:2554 + break; +case llvm::omp::OMPC_adjust_args: +case llvm::omp::OMPC_affinity: I think all these cases are unexpected and must be terminated with llvm_unreachable

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-10 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 558076. koops added a comment. Removing default from switch statements. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/OpenMPClause.h clang/include/clang/AST/RecursiveASTV

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-08 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D123235#4656464 , @koops wrote: > In clang/lib/AST/OpenMPClause.cpp, > > OMPClauseWithPreInit::get(const OMPClause *C) { > switch(C->getClauseKind()) { > case OMPC_schedule: > > default: > break; > } > >

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-08 Thread Sunil K via Phabricator via cfe-commits
koops added a comment. In clang/lib/AST/OpenMPClause.cpp, OMPClauseWithPreInit::get(const OMPClause *C) { switch(C->getClauseKind()) { case OMPC_schedule: default: break; } It is not possible to list down all possible ``OpenMPClauseKind``` types in the switch. Hence they

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-08 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:2555 + break; +default: + this->FailParameter = llvm::omp::OMPC_unknown; Same here, remove default: and explicitly list all possible values CHANGES SINCE LAST ACTION

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-08 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D123235#4656459 , @koops wrote: > After the reverting of changes, putting the default in the switch case as > discussed in the comments earlier. No, do not put the default: switch, instead you need explicitly list all other

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-08 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 558056. koops added a comment. After the rollback putting the default in the switch case as discussed in the comments earlier. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AS

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-08 Thread Sunil K via Phabricator via cfe-commits
koops added inline comments. Comment at: clang/lib/Basic/OpenMPKinds.cpp:450 +case OMPC_unknown: +default: + return "unknown"; uabelho wrote: > Adding "default:" here silences the warning. > But looks like @ABataev commented on something similar earl

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-08 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Btw, a bit confusing (to me at least) to reopen this review since it's already landed. Comment at: clang/lib/Basic/OpenMPKinds.cpp:450 +case OMPC_unknown: +default: + return "unknown"; Adding "default:" here silences the w

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-08 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D123235#4656427 , @koops wrote: > To avoid build error in ppc64 adding a "default" to switch statement. I don't think it has anything particular to do with ppc64. I see the warning/error when i compile with clang 15 on a RHEL

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-08 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 558051. koops added a comment. To avoid build error in ppc64 adding a "default" to switch statement. This is similar to the way it is currently handled in OMPClauseWithPreInit::get() { switch(C->getClauseKind()) { case ... default: } CHANG

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-08 Thread Sunil K via Phabricator via cfe-commits
koops added a comment. Only on ppc64 this error is being seen because it is expecting a default in the switch statement: OpenMPClauseKind CK = ... switch(CK) This is similar to what has been added in OMPClauseWithPreInit::get() { switch(C->getClauseKind()) { case ... default: } I am about t

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-08 Thread Qiu Chaofan via Phabricator via cfe-commits
qiucf added a comment. Hi, it seems rG086b65340cca2648a2a91a0a47d28c7d9bafd1e5 causes build failure: llvm-project/clang/lib/Basic/OpenMPKinds.cpp:444:13: error: 97 enumeration values not handled in switch: 'OMPC_adjust_arg

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-07 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang/lib/Basic/OpenMPKinds.cpp:444 +OpenMPClauseKind CK = static_cast(Type); +switch (CK) { +case OMPC_unknown: Hi @koops , clang complains on this patch with ``` ../../clang/lib/Basic/OpenMPKinds.cpp:444:13

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-07 Thread Chi Chun Chen via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG086b65340cca: [OpenMP] atomic compare fail : Parser & AST support (authored by koops, committed by cchen). Repository: rG LLVM Github Monorepo CH

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-07 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 558041. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/OpenMPClause.h clang/include/clang/AST/RecursiveASTVisitor.h clang/include/clang/Basic/DiagnosticSemaKinds.td clan

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-07 Thread Alexey Bataev via Phabricator via cfe-commits
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/Sema/SemaOpenMP.cpp:12693 + OpenMPClauseKind FailParameter = FC->getFailParameter(); + SourceLocation DisplayLocation = (FailP

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-07 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 558040. koops added a comment. Removing extra brackets. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/OpenMPClause.h clang/include/clang/AST/RecursiveASTVisitor.h clang

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-07 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:12694-12695 + SourceLocation DisplayLocation = + ((FailParameter == OMPC_unknown) ? FC->getBeginLoc() + : FC->getFailParameterLoc()); + if ((FailP

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-07 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 558039. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/OpenMPClause.h clang/include/clang/AST/RecursiveASTVisitor.h clang/include/clang/Basic/DiagnosticSemaKinds.td clan

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-07 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:3837-3838 return nullptr; - return Actions.ActOnOpenMPClause(Kind, Loc, Tok.getLocation()); + OMPClause *Clause = Actions.ActOnOpenMPClause(Kind, Loc, Tok.getLocation()); + return Clause; } --

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-04 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 558007. koops added a comment. 1. Removing the Create methods from OMPFailClause class. 2. checkFailClauseParameters removed and the checking is now done in ActOnOpenMPAtomicDirective() itself. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-02 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:2561 + + void initFailClause(SourceLocation LParenLoc, OpenMPClauseKind FailParameter, + SourceLocation FailParameterLoc) { Unused? Comment

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-02 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 557992. koops added a comment. Moving OMC_fail to use ParseOpenMPSimpleClause instead of ParseOpenMPClause. Other changes suggested by previous review. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files:

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-10-30 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:3328-3332 if (!FirstClause) { Diag(Tok, diag::err_omp_more_one_clause) << getOpenMPDirectiveName(DKind) << getOpenMPClauseName(CKind) << 0; ErrorFound = true; } -

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-10-30 Thread Sunil K via Phabricator via cfe-commits
koops added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:2611-2617 + void initFailClause(SourceLocation LParenLoc, OpenMPClauseKind FailParameter, + SourceLocation FailParameterLoc) { + +setLParenLoc(LParenLoc); +setFailParamet

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-10-30 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:2543 + void setFailParameter(OpenMPClauseKind FailParameter) { + +switch (FailParameter) { Remove this empty line Comment at: clang/include/clang/AST/OpenMPC

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-10-30 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 557933. koops added a comment. Replacing the storing of "OMPClause *FailMemoryOrderClause" with "OpenMPClauseKind FailParameter" in class OMPFailClause. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-10-28 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:2525 + + OMPClause *FailMemoryOrderClause = nullptr; + SourceLocation ArgumentLoc; I don't like the idea of a reference to another clause here, it may lead to many issues with se

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-10-27 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 557920. koops added a comment. Removing the printing of parameter of FailClause. This needed a special if statement in Visit(const OMPClause *C) , which is a generalized Visit for Clauses. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ ht

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-10-20 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 557820. koops added a comment. Correcting a git-clang-format error. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/ASTNodeTraverser.h clang/include/clang/AST/OpenMPClause.

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-10-20 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 557808. koops added a comment. 1. In class OMPFailClause, the was a duplication in the storage of parameter to the fail clause because the parameter was stored as FailParameterKind and MemoryOrderClause (FailMemoryOrderClause). There was a possibility of these

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-10-18 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/include/clang/AST/ASTNodeTraverser.h:217-220 +if (const auto *OMPC = dyn_cast(C)) { + Visit(OMPC); + return; +} Why do you need special logic here? Comment at: clang/include/cla

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-10-18 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 557749. koops added a comment. Removed the tail-allocation. Other changes suggested in the previous review. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/ASTNodeTraverser.h

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-08-10 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:2320-2321 +: public OMPClause, + private llvm::TrailingObjects { + OMPClause *MemoryOrderClause; Why need tail-allocation here for constant number of attributes? They can

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-08-09 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 548886. koops added a comment. Corrected git-clang-format errors. clang/include/clang/AST/OpenMPClause.h clang/lib/AST/OpenMPClause.cpp clang/lib/Parse/ParseOpenMP.cpp clang/lib/Sema/SemaOpenMP.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-08-09 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 548534. koops added a comment. Pulling in latest changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/ASTNodeTraverser.h clang/include/clang/AST/OpenMPClause.h clang/

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-08-09 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 548494. koops added a reviewer: thakis. koops added a comment. Making the CHECK pattern generic to match platforms tested after committing changes (especially MacOS). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-08-08 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 548103. Herald added a subscriber: jplehr. Herald added a reviewer: kiranchandramohan. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/ASTNodeTraverser.h clang/include/clang/

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-06-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Precommit CI on Linux seems to be failing, but I can't quite tell whether the failures are related or not. I suspect they're unrelated, but they're with libomp and other OpenMP tests so it's not fully clear. Can you investigate those? I tried your patch locally o

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-06-10 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 435831. koops added a comment. Changes suggested by aaron.ballman to avoid failures on windows. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/ASTNodeTraverser.h clang/inc

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-06-08 Thread Sunil K via Phabricator via cfe-commits
koops added a comment. Is it possible to move this back from "Closed" state to "Needs Review"? The merge has been rolled back due to a defect in the code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D123235#3537820 , @aaron.ballman wrote: > In D123235#3537636 , @aaron.ballman > wrote: > >> Please revert the changes while investigating how to resolve the crashes >> though.

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D123235#3537636 , @aaron.ballman wrote: > Please revert the changes while investigating how to resolve the crashes > though. I just noticed that you needed someone to commit on your behalf for this, so I went ahead an

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D123235#3537580 , @koops wrote: > I am not sure why it is indicating an uninitialized variable at that point. > The code at " llvm-project/clang/lib/Sema/SemaOpenMP.cpp:12060 " is as below: FWIW, it's outright crashing

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-25 Thread Sunil K via Phabricator via cfe-commits
koops added subscribers: cchen, dreachem, tianshilei1992, jdoerfert, soumitra. koops added a comment. Hello Kevin, I am not sure why it is indicating an uninitialized variable at that point. The code at " llvm-project/clang/lib/Sema/SemaOpenMP.cpp:12060 " is as below: for (const OMPClause *C

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-25 Thread Kevin Athey via Phabricator via cfe-commits
kda added a comment. This has broken sanitizer buildbot: https://lab.llvm.org/buildbot/#/builders/5/builds/24074 Relevant log snippet: FAIL: Clang :: OpenMP/atomic_messages.cpp (9696 of 66080) TEST 'Clang :: OpenMP/atomic_messages.cpp' FAILED Scri

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-24 Thread Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG232bf8189ef7: [OpenMP] atomic compare fail : Parser & AST support (authored by koops, committed by Chi-Chun, Chen ). Heral

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-23 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 accepted this revision. tianshilei1992 added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/Parse/ParseOpenMP.cpp:3692-3694 + if (Kind == llvm::omp::Clause::OMPC_fail) { +Clause = ParseOpenMPFailClause(Clause); + } --

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-23 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 431391. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/ASTNodeTraverser.h clang/include/clang/AST/OpenMPClause.h clang/include/clang/AST/RecursiveASTVisitor.h clang/incl

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-23 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 431385. koops added a comment. Clang formatting for variables in ParseOpenMP.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/ASTNodeTraverser.h clang/include/clang/AST/

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-23 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 431315. koops added a comment. Fixing a minor error : clang formatting of variable names to avoid build errors. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/ASTNodeTravers

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-20 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 added a comment. LGTM, but please fix the build error first. Comment at: clang/include/clang/AST/ASTNodeTraverser.h:228 + void Visit(const OMPFailClause *C) { +getNodeDelegate().AddChild([=] { koops wrote: > tianshilei1992 wrote: > > koops

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-19 Thread Sunil K via Phabricator via cfe-commits
koops added inline comments. Comment at: clang/include/clang/AST/ASTNodeTraverser.h:228 + void Visit(const OMPFailClause *C) { +getNodeDelegate().AddChild([=] { tianshilei1992 wrote: > koops wrote: > > tianshilei1992 wrote: > > > Why would we want a dedica

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-19 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 430883. koops added a comment. Further changes in the code to confirm to the clang format. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/ASTNodeTraverser.h clang/include/

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-14 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 added a comment. In D123235#3513430 , @koops wrote: > I have tried on x64 RH and x64 SuSe. I could not reproduce the failures seen > on x64 debian. https://reviews.llvm.org/D118550 also has similar failures on > x64 debian. There is a co

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-14 Thread Sunil K via Phabricator via cfe-commits
koops added a comment. I have tried on x64 RH and x64 SuSe. I could not reproduce the failures seen on x64 debian. https://reviews.llvm.org/D118550 also has similar failures on x64 debian. There is a comment " I think the test failures are spurious (but not 100% sure)" So, are these failures

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-13 Thread Sunil K via Phabricator via cfe-commits
koops added inline comments. Comment at: clang/include/clang/AST/ASTNodeTraverser.h:228 + void Visit(const OMPFailClause *C) { +getNodeDelegate().AddChild([=] { tianshilei1992 wrote: > Why would we want a dedicated function since it is only called once? Th

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-13 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 429285. koops added a comment. Took care of the clang format. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 Files: clang/include/clang/AST/ASTNodeTraverser.h clang/include/clang/AST/OpenMPClause.h cl

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-02 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 added inline comments. Comment at: clang/include/clang/AST/ASTNodeTraverser.h:217 void Visit(const OMPClause *C) { +if(OMPFailClause::classof(C)) { + Visit(static_cast(C)); clang-format plz. Comment at: clang/include/

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-04-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. FWIW, I changed the patch summary as a drive-by because I originally thought this was adding a global pragma named `atomic compare fail` and was very confused. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 __