tianshilei1992 added a comment. In D123235#3513430 <https://reviews.llvm.org/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 comment " I think the test failures are spurious (but > not 100% sure)" So, are these failures pre-existing before the changes in > the current support for "atomic compare fail: Parser & Support" were done? Those tests should be irrelevant. ================ Comment at: clang/include/clang/AST/ASTNodeTraverser.h:228 + void Visit(const OMPFailClause *C) { + getNodeDelegate().AddChild([=] { ---------------- koops wrote: > tianshilei1992 wrote: > > Why would we want a dedicated function since it is only called once? > The code for this method cannot be put into any other method because it > handles only OMPFailClause. All other Visit methods handle either the > generalized OMPClause or other types of Clauses. I mean, it's only used by the function above, no? ================ Comment at: clang/include/clang/AST/ASTNodeTraverser.h:231-232 + getNodeDelegate().Visit(C); + const OMPClause *mOC = C->const_getMemoryOrderClause(); + Visit(mOC); + }); ---------------- ================ Comment at: clang/include/clang/Parse/Parser.h:434 + OMPClause *ParseOpenMPFailClause(OMPClause *clause); + ---------------- ================ Comment at: clang/lib/AST/OpenMPClause.cpp:417-432 +OMPFailClause *OMPFailClause::Create(const ASTContext &C, + SourceLocation StartLoc, + SourceLocation EndLoc) { + void *Mem = + C.Allocate(totalSizeToAlloc<SourceLocation, OpenMPClauseKind>(2, 1), alignof(OMPFailClause)); + auto *Clause = + new (Mem) OMPFailClause(StartLoc, EndLoc); ---------------- clang-format plz ================ Comment at: clang/lib/Basic/OpenMPKinds.cpp:370 + case OMPC_fail: { + OpenMPClauseKind ck = static_cast<OpenMPClauseKind>(Type); + switch (ck) { ---------------- maybe something like `CK`? `ck` doesn't conform with LLVM standard. ================ Comment at: clang/lib/Parse/ParseOpenMP.cpp:3622 + + OMPFailClause *failClause = static_cast<OMPFailClause *>(clause); + SourceLocation LParenLoc; ---------------- ================ Comment at: clang/lib/Parse/ParseOpenMP.cpp:3691 return nullptr; - return Actions.ActOnOpenMPClause(Kind, Loc, Tok.getLocation()); + OMPClause *clause = Actions.ActOnOpenMPClause(Kind, Loc, Tok.getLocation()); + if (Kind == llvm::omp::Clause::OMPC_fail) { ---------------- ditto ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:11999 + SourceLocation *ErrorLoc) { + int no_of_fails = 0; + ErrorNo = 0; ---------------- ditto CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D123235 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
