llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules Author: Amit Tiwari (loopacino) <details> <summary>Changes</summary> Use a normal constructor when the clause has a fixed size. Keep `Create/CreateEmpty` only when the object needs extra tail storage. Covers: `looprange, full, partial, bind, depobj, align` --- Full diff: https://github.com/llvm/llvm-project/pull/224935.diff 4 Files Affected: - (modified) clang/include/clang/AST/OpenMPClause.h (+32-88) - (modified) clang/lib/AST/OpenMPClause.cpp (-82) - (modified) clang/lib/Sema/SemaOpenMP.cpp (+11-11) - (modified) clang/lib/Serialization/ASTReader.cpp (+5-5) ``````````diff diff --git a/clang/include/clang/AST/OpenMPClause.h b/clang/include/clang/AST/OpenMPClause.h index ec84f10956ff4..2884afb666e12 100644 --- a/clang/include/clang/AST/OpenMPClause.h +++ b/clang/include/clang/AST/OpenMPClause.h @@ -441,6 +441,7 @@ class OMPAlignClause final /// Set alignment value. void setAlignment(Expr *A) { setStmt(A); } +public: /// Build 'align' clause with the given alignment /// /// \param A Alignment value. @@ -454,18 +455,6 @@ class OMPAlignClause final /// Build an empty clause. OMPAlignClause() : OMPOneStmtClause() {} -public: - /// Build 'align' clause with the given alignment - /// - /// \param A Alignment value. - /// \param StartLoc Starting location of the clause. - /// \param LParenLoc Location of '('. - /// \param EndLoc Ending location of the clause. - static OMPAlignClause *Create(const ASTContext &C, Expr *A, - SourceLocation StartLoc, - SourceLocation LParenLoc, - SourceLocation EndLoc); - /// Returns alignment Expr *getAlignment() const { return getStmtAs<Expr>(); } }; @@ -1331,24 +1320,16 @@ class OMPPermutationClause final /// for (int i = 0; i < 64; ++i) /// \endcode class OMPFullClause final : public OMPNoChildClause<llvm::omp::OMPC_full> { - friend class OMPClauseReader; - - /// Build an empty clause. - explicit OMPFullClause() : OMPNoChildClause() {} - public: /// Build an AST node for a 'full' clause. /// - /// \param C Context of the AST. /// \param StartLoc Starting location of the clause. /// \param EndLoc Ending location of the clause. - static OMPFullClause *Create(const ASTContext &C, SourceLocation StartLoc, - SourceLocation EndLoc); + OMPFullClause(SourceLocation StartLoc, SourceLocation EndLoc) + : OMPNoChildClause(StartLoc, EndLoc) {} - /// Build an empty 'full' AST node for deserialization. - /// - /// \param C Context of the AST. - static OMPFullClause *CreateEmpty(const ASTContext &C); + /// Build an empty clause. + explicit OMPFullClause() : OMPNoChildClause() {} }; /// This class represents the 'looprange' clause in the @@ -1379,20 +1360,21 @@ class OMPLoopRangeClause final : public OMPClause { /// Set looprange 'count' expression void setCount(Expr *E) { Args[CountExpr] = E; } +public: + /// Build a 'looprange' clause. + OMPLoopRangeClause(SourceLocation StartLoc, SourceLocation LParenLoc, + SourceLocation FirstLoc, SourceLocation CountLoc, + SourceLocation EndLoc, Expr *First, Expr *Count) + : OMPClause(llvm::omp::OMPC_looprange, StartLoc, EndLoc), + LParenLoc(LParenLoc), FirstLoc(FirstLoc), CountLoc(CountLoc) { + setFirst(First); + setCount(Count); + } + /// Build an empty clause for deserialization. explicit OMPLoopRangeClause() : OMPClause(llvm::omp::OMPC_looprange, {}, {}) {} -public: - /// Build a 'looprange' clause AST node. - static OMPLoopRangeClause * - Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation LParenLoc, - SourceLocation FirstLoc, SourceLocation CountLoc, - SourceLocation EndLoc, Expr *First, Expr *Count); - - /// Build an empty 'looprange' clause node. - static OMPLoopRangeClause *CreateEmpty(const ASTContext &C); - // Location getters/setters SourceLocation getLParenLoc() const { return LParenLoc; } SourceLocation getFirstLoc() const { return FirstLoc; } @@ -1439,10 +1421,7 @@ class OMPPartialClause final : public OMPClause { SourceLocation LParenLoc; /// Optional argument to the clause (unroll factor). - Stmt *Factor; - - /// Build an empty clause. - explicit OMPPartialClause() : OMPClause(llvm::omp::OMPC_partial, {}, {}) {} + Stmt *Factor = nullptr; /// Set the unroll factor. void setFactor(Expr *E) { Factor = E; } @@ -1453,19 +1432,17 @@ class OMPPartialClause final : public OMPClause { public: /// Build an AST node for a 'partial' clause. /// - /// \param C Context of the AST. /// \param StartLoc Location of the 'partial' identifier. /// \param LParenLoc Location of '('. /// \param EndLoc Location of ')'. /// \param Factor Clause argument. - static OMPPartialClause *Create(const ASTContext &C, SourceLocation StartLoc, - SourceLocation LParenLoc, - SourceLocation EndLoc, Expr *Factor); + OMPPartialClause(SourceLocation StartLoc, SourceLocation LParenLoc, + SourceLocation EndLoc, Expr *Factor) + : OMPClause(llvm::omp::OMPC_partial, StartLoc, EndLoc), + LParenLoc(LParenLoc), Factor(Factor) {} - /// Build an empty 'partial' AST node for deserialization. - /// - /// \param C Context of the AST. - static OMPPartialClause *CreateEmpty(const ASTContext &C); + /// Build an empty clause. + explicit OMPPartialClause() : OMPClause(llvm::omp::OMPC_partial, {}, {}) {} /// Returns the location of '('. SourceLocation getLParenLoc() const { return LParenLoc; } @@ -5721,42 +5698,26 @@ class OMPDepobjClause final : public OMPClause { /// Chunk size. Expr *Depobj = nullptr; - /// Build clause with number of variables \a N. - /// - /// \param StartLoc Starting location of the clause. - /// \param LParenLoc Location of '('. - /// \param EndLoc Ending location of the clause. - OMPDepobjClause(SourceLocation StartLoc, SourceLocation LParenLoc, - SourceLocation EndLoc) - : OMPClause(llvm::omp::OMPC_depobj, StartLoc, EndLoc), - LParenLoc(LParenLoc) {} - - /// Build an empty clause. - /// - explicit OMPDepobjClause() - : OMPClause(llvm::omp::OMPC_depobj, SourceLocation(), SourceLocation()) {} - void setDepobj(Expr *E) { Depobj = E; } /// Sets the location of '('. void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; } public: - /// Creates clause. + /// Build a 'depobj' clause. /// - /// \param C AST context. /// \param StartLoc Starting location of the clause. /// \param LParenLoc Location of '('. /// \param EndLoc Ending location of the clause. /// \param Depobj depobj expression associated with the 'depobj' directive. - static OMPDepobjClause *Create(const ASTContext &C, SourceLocation StartLoc, - SourceLocation LParenLoc, - SourceLocation EndLoc, Expr *Depobj); + OMPDepobjClause(SourceLocation StartLoc, SourceLocation LParenLoc, + SourceLocation EndLoc, Expr *Depobj) + : OMPClause(llvm::omp::OMPC_depobj, StartLoc, EndLoc), + LParenLoc(LParenLoc), Depobj(Depobj) {} - /// Creates an empty clause. - /// - /// \param C AST context. - static OMPDepobjClause *CreateEmpty(const ASTContext &C); + /// Build an empty clause. + explicit OMPDepobjClause() + : OMPClause(llvm::omp::OMPC_depobj, SourceLocation(), SourceLocation()) {} /// Returns depobj expression associated with the clause. Expr *getDepobj() { return Depobj; } @@ -9945,6 +9906,7 @@ class OMPBindClause final : public OMPNoChildClause<llvm::omp::OMPC_bind> { /// Set the binding kind location. void setBindKindLoc(SourceLocation KLoc) { KindLoc = KLoc; } +public: /// Build 'bind' clause with kind \a K ('teams', 'parallel', or 'thread'). /// /// \param K Binding kind of the clause ('teams', 'parallel' or 'thread'). @@ -9961,24 +9923,6 @@ class OMPBindClause final : public OMPNoChildClause<llvm::omp::OMPC_bind> { /// Build an empty clause. OMPBindClause() : OMPNoChildClause() {} -public: - /// Build 'bind' clause with kind \a K ('teams', 'parallel', or 'thread'). - /// - /// \param C AST context - /// \param K Binding kind of the clause ('teams', 'parallel' or 'thread'). - /// \param KLoc Starting location of the binding kind. - /// \param StartLoc Starting location of the clause. - /// \param LParenLoc Location of '('. - /// \param EndLoc Ending location of the clause. - static OMPBindClause *Create(const ASTContext &C, OpenMPBindClauseKind K, - SourceLocation KLoc, SourceLocation StartLoc, - SourceLocation LParenLoc, SourceLocation EndLoc); - - /// Build an empty 'bind' clause. - /// - /// \param C AST context - static OMPBindClause *CreateEmpty(const ASTContext &C); - /// Returns the location of '('. SourceLocation getLParenLoc() const { return LParenLoc; } diff --git a/clang/lib/AST/OpenMPClause.cpp b/clang/lib/AST/OpenMPClause.cpp index 2061d5395ac65..bf641bf1ba7ee 100644 --- a/clang/lib/AST/OpenMPClause.cpp +++ b/clang/lib/AST/OpenMPClause.cpp @@ -654,13 +654,6 @@ OMPAlignedClause *OMPAlignedClause::CreateEmpty(const ASTContext &C, return new (Mem) OMPAlignedClause(NumVars); } -OMPAlignClause *OMPAlignClause::Create(const ASTContext &C, Expr *A, - SourceLocation StartLoc, - SourceLocation LParenLoc, - SourceLocation EndLoc) { - return new (C) OMPAlignClause(A, StartLoc, LParenLoc, EndLoc); -} - void OMPCopyinClause::setSourceExprs(ArrayRef<Expr *> SrcExprs) { assert(SrcExprs.size() == varlist_size() && "Number of source expressions is " "not the same as the " @@ -1017,56 +1010,6 @@ OMPPermutationClause *OMPPermutationClause::CreateEmpty(const ASTContext &C, return new (Mem) OMPPermutationClause(NumLoops); } -OMPFullClause *OMPFullClause::Create(const ASTContext &C, - SourceLocation StartLoc, - SourceLocation EndLoc) { - OMPFullClause *Clause = CreateEmpty(C); - Clause->setLocStart(StartLoc); - Clause->setLocEnd(EndLoc); - return Clause; -} - -OMPFullClause *OMPFullClause::CreateEmpty(const ASTContext &C) { - return new (C) OMPFullClause(); -} - -OMPPartialClause *OMPPartialClause::Create(const ASTContext &C, - SourceLocation StartLoc, - SourceLocation LParenLoc, - SourceLocation EndLoc, - Expr *Factor) { - OMPPartialClause *Clause = CreateEmpty(C); - Clause->setLocStart(StartLoc); - Clause->setLParenLoc(LParenLoc); - Clause->setLocEnd(EndLoc); - Clause->setFactor(Factor); - return Clause; -} - -OMPPartialClause *OMPPartialClause::CreateEmpty(const ASTContext &C) { - return new (C) OMPPartialClause(); -} - -OMPLoopRangeClause * -OMPLoopRangeClause::Create(const ASTContext &C, SourceLocation StartLoc, - SourceLocation LParenLoc, SourceLocation FirstLoc, - SourceLocation CountLoc, SourceLocation EndLoc, - Expr *First, Expr *Count) { - OMPLoopRangeClause *Clause = CreateEmpty(C); - Clause->setLocStart(StartLoc); - Clause->setLParenLoc(LParenLoc); - Clause->setFirstLoc(FirstLoc); - Clause->setCountLoc(CountLoc); - Clause->setLocEnd(EndLoc); - Clause->setFirst(First); - Clause->setCount(Count); - return Clause; -} - -OMPLoopRangeClause *OMPLoopRangeClause::CreateEmpty(const ASTContext &C) { - return new (C) OMPLoopRangeClause(); -} - OMPAllocateClause *OMPAllocateClause::Create( const ASTContext &C, SourceLocation StartLoc, SourceLocation LParenLoc, Expr *Allocator, Expr *Alignment, SourceLocation ColonLoc, @@ -1107,20 +1050,6 @@ OMPFlushClause *OMPFlushClause::CreateEmpty(const ASTContext &C, unsigned N) { return new (Mem) OMPFlushClause(N); } -OMPDepobjClause *OMPDepobjClause::Create(const ASTContext &C, - SourceLocation StartLoc, - SourceLocation LParenLoc, - SourceLocation RParenLoc, - Expr *Depobj) { - auto *Clause = new (C) OMPDepobjClause(StartLoc, LParenLoc, RParenLoc); - Clause->setDepobj(Depobj); - return Clause; -} - -OMPDepobjClause *OMPDepobjClause::CreateEmpty(const ASTContext &C) { - return new (C) OMPDepobjClause(); -} - OMPDependClause * OMPDependClause::Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation LParenLoc, SourceLocation EndLoc, @@ -1850,17 +1779,6 @@ void OMPInitClause::setAttrs(ArrayRef<unsigned> Counts, llvm::copy(Attrs, getTrailingObjects<Expr *>() + varlist_size()); } -OMPBindClause * -OMPBindClause::Create(const ASTContext &C, OpenMPBindClauseKind K, - SourceLocation KLoc, SourceLocation StartLoc, - SourceLocation LParenLoc, SourceLocation EndLoc) { - return new (C) OMPBindClause(K, KLoc, StartLoc, LParenLoc, EndLoc); -} - -OMPBindClause *OMPBindClause::CreateEmpty(const ASTContext &C) { - return new (C) OMPBindClause(); -} - OMPDoacrossClause * OMPDoacrossClause::Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation LParenLoc, SourceLocation EndLoc, diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index 2e4d9f2f82f0b..6c46cd547592a 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -18589,7 +18589,7 @@ OMPClause *SemaOpenMP::ActOnOpenMPPermutationClause(ArrayRef<Expr *> PermExprs, OMPClause *SemaOpenMP::ActOnOpenMPFullClause(SourceLocation StartLoc, SourceLocation EndLoc) { - return OMPFullClause::Create(getASTContext(), StartLoc, EndLoc); + return new (getASTContext()) OMPFullClause(StartLoc, EndLoc); } OMPClause *SemaOpenMP::ActOnOpenMPPartialClause(Expr *FactorExpr, @@ -18606,8 +18606,8 @@ OMPClause *SemaOpenMP::ActOnOpenMPPartialClause(Expr *FactorExpr, FactorExpr = FactorResult.get(); } - return OMPPartialClause::Create(getASTContext(), StartLoc, LParenLoc, EndLoc, - FactorExpr); + return new (getASTContext()) + OMPPartialClause(StartLoc, LParenLoc, EndLoc, FactorExpr); } OMPClause *SemaOpenMP::ActOnOpenMPLoopRangeClause( @@ -18631,8 +18631,8 @@ OMPClause *SemaOpenMP::ActOnOpenMPLoopRangeClause( // loop sequence length of the associated canonical loop sequence. // This check must be performed afterwards due to the delayed // parsing and computation of the associated loop sequence - return OMPLoopRangeClause::Create(getASTContext(), StartLoc, LParenLoc, - FirstLoc, CountLoc, EndLoc, First, Count); + return new (getASTContext()) OMPLoopRangeClause( + StartLoc, LParenLoc, FirstLoc, CountLoc, EndLoc, First, Count); } OMPClause *SemaOpenMP::ActOnOpenMPAlignClause(Expr *A, SourceLocation StartLoc, @@ -18642,8 +18642,8 @@ OMPClause *SemaOpenMP::ActOnOpenMPAlignClause(Expr *A, SourceLocation StartLoc, AlignVal = VerifyPositiveIntegerConstantInClause(A, OMPC_align); if (AlignVal.isInvalid()) return nullptr; - return OMPAlignClause::Create(getASTContext(), AlignVal.get(), StartLoc, - LParenLoc, EndLoc); + return new (getASTContext()) + OMPAlignClause(AlignVal.get(), StartLoc, LParenLoc, EndLoc); } OMPClause *SemaOpenMP::ActOnOpenMPSingleExprWithArgClause( @@ -22427,8 +22427,8 @@ OMPClause *SemaOpenMP::ActOnOpenMPDepobjClause(Expr *Depobj, << 1 << Depobj->getSourceRange(); } - return OMPDepobjClause::Create(getASTContext(), StartLoc, LParenLoc, EndLoc, - Depobj); + return new (getASTContext()) + OMPDepobjClause(StartLoc, LParenLoc, EndLoc, Depobj); } namespace { @@ -26481,8 +26481,8 @@ OMPClause *SemaOpenMP::ActOnOpenMPBindClause(OpenMPBindClauseKind Kind, return nullptr; } - return OMPBindClause::Create(getASTContext(), Kind, KindLoc, StartLoc, - LParenLoc, EndLoc); + return new (getASTContext()) + OMPBindClause(Kind, KindLoc, StartLoc, LParenLoc, EndLoc); } OMPClause *SemaOpenMP::ActOnOpenMPXDynCGroupMemClause(Expr *Size, diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index a9c230d767c50..76b3ecb8f96db 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -11503,13 +11503,13 @@ OMPClause *OMPClauseReader::readClause() { break; } case llvm::omp::OMPC_full: - C = OMPFullClause::CreateEmpty(Context); + C = new (Context) OMPFullClause(); break; case llvm::omp::OMPC_partial: - C = OMPPartialClause::CreateEmpty(Context); + C = new (Context) OMPPartialClause(); break; case llvm::omp::OMPC_looprange: - C = OMPLoopRangeClause::CreateEmpty(Context); + C = new (Context) OMPLoopRangeClause(); break; case llvm::omp::OMPC_allocator: C = new (Context) OMPAllocatorClause(); @@ -11684,7 +11684,7 @@ OMPClause *OMPClauseReader::readClause() { C = OMPFlushClause::CreateEmpty(Context, Record.readInt()); break; case llvm::omp::OMPC_depobj: - C = OMPDepobjClause::CreateEmpty(Context); + C = new (Context) OMPDepobjClause(); break; case llvm::omp::OMPC_depend: { unsigned NumVars = Record.readInt(); @@ -11829,7 +11829,7 @@ OMPClause *OMPClauseReader::readClause() { C = new (Context) OMPFilterClause(); break; case llvm::omp::OMPC_bind: - C = OMPBindClause::CreateEmpty(Context); + C = new (Context) OMPBindClause(); break; case llvm::omp::OMPC_align: C = new (Context) OMPAlignClause(); `````````` </details> https://github.com/llvm/llvm-project/pull/224935 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
