Re: r270009 - Make Sema::getPrintingPolicy less ridiculously expensive. This used to perform

[Alexander Potapenko via cfe-commits]

Pretty impressive, thank you! I'm gonna give it a shot later today.

sent from phone
On May 19, 2016 3:45 AM, "Richard Smith via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Wed May 18 20:39:10 2016
> New Revision: 270009
>
> URL: http://llvm.org/viewvc/llvm-project?rev=270009=rev
> Log:
> Make Sema::getPrintingPolicy less ridiculously expensive. This used to
> perform
> an identifier table lookup, *and* copy the LangOptions (including various
> std::vectors). Twice. We call this function once each time we
> start
> parsing a declaration specifier sequence, and once for each call to
> Sema::Diag.
>
> This reduces the compile time for a sample .c file from the linux kernel
> by 20%.
>
> Modified:
> cfe/trunk/include/clang/AST/ASTContext.h
> cfe/trunk/include/clang/AST/PrettyPrinter.h
> cfe/trunk/lib/AST/DeclarationName.cpp
> cfe/trunk/lib/AST/StmtPrinter.cpp
> cfe/trunk/lib/AST/TypePrinter.cpp
> cfe/trunk/lib/Parse/ParseDecl.cpp
> cfe/trunk/lib/Sema/Sema.cpp
> cfe/trunk/lib/Sema/SemaCodeComplete.cpp
> cfe/trunk/test/Analysis/initializers-cfg-output.cpp
> cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp
> cfe/trunk/test/SemaCXX/member-pointer.cpp
>
> Modified: cfe/trunk/include/clang/AST/ASTContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=270009=270008=270009=diff
>
> ==
> --- cfe/trunk/include/clang/AST/ASTContext.h (original)
> +++ cfe/trunk/include/clang/AST/ASTContext.h Wed May 18 20:39:10 2016
> @@ -243,6 +243,9 @@ class ASTContext : public RefCountedBase
>QualType ObjCClassRedefinitionType;
>QualType ObjCSelRedefinitionType;
>
> +  /// The identifier 'bool'.
> +  mutable IdentifierInfo *BoolName = nullptr;
> +
>/// The identifier 'NSObject'.
>IdentifierInfo *NSObjectName = nullptr;
>
> @@ -1457,6 +1460,13 @@ public:
>  return NSCopyingName;
>}
>
> +  /// Retrieve the identifier 'bool'.
> +  IdentifierInfo *getBoolName() const {
> +if (!BoolName)
> +  BoolName = ("bool");
> +return BoolName;
> +  }
> +
>IdentifierInfo *getMakeIntegerSeqName() const {
>  if (!MakeIntegerSeqName)
>MakeIntegerSeqName = ("__make_integer_seq");
>
> Modified: cfe/trunk/include/clang/AST/PrettyPrinter.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/PrettyPrinter.h?rev=270009=270008=270009=diff
>
> ==
> --- cfe/trunk/include/clang/AST/PrettyPrinter.h (original)
> +++ cfe/trunk/include/clang/AST/PrettyPrinter.h Wed May 18 20:39:10 2016
> @@ -32,22 +32,35 @@ public:
>
>  /// \brief Describes how types, statements, expressions, and
>  /// declarations should be printed.
> +///
> +/// This type is intended to be small and suitable for passing by value.
> +/// It is very frequently copied.
>  struct PrintingPolicy {
> -  /// \brief Create a default printing policy for C.
> +  /// \brief Create a default printing policy for the specified language.
>PrintingPolicy(const LangOptions )
> -: LangOpts(LO), Indentation(2), SuppressSpecifiers(false),
> -  SuppressTagKeyword(false),
> +: Indentation(2), SuppressSpecifiers(false),
> +  SuppressTagKeyword(LO.CPlusPlus),
>IncludeTagDefinition(false), SuppressScope(false),
>SuppressUnwrittenScope(false), SuppressInitializers(false),
>ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
>SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
>SuppressTemplateArgsInCXXConstructors(false),
> -  Bool(LO.Bool), TerseOutput(false), PolishForDeclaration(false),
> +  Bool(LO.Bool), Restrict(LO.C99),
> +  Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
> +  UseVoidForZeroParams(!LO.CPlusPlus),
> +  TerseOutput(false), PolishForDeclaration(false),
>Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
>IncludeNewlines(true), MSVCFormatting(false) { }
>
> -  /// \brief What language we're printing.
> -  LangOptions LangOpts;
> +  /// \brief Adjust this printing policy for cases where it's known that
> +  /// we're printing C++ code (for instance, if AST dumping reaches a
> +  /// C++-only construct). This should not be used if a real LangOptions
> +  /// object is available.
> +  void adjustForCPlusPlus() {
> +SuppressTagKeyword = true;
> +Bool = true;
> +UseVoidForZeroParams = false;
> +  }
>
>/// \brief The number of spaces to use to indent each line.
>unsigned Indentation : 8;
> @@ -143,10 +156,23 @@ struct PrintingPolicy {
>/// constructors.
>unsigned SuppressTemplateArgsInCXXConstructors : 1;
>
> -  /// \brief Whether we can use 'bool' rather than '_Bool', even if the
> language
> -  /// doesn't actually have 'bool' (because, e.g., it is defined as a
> macro).
> +  /// \brief Whether we can use 

Re: r270009 - Make Sema::getPrintingPolicy less ridiculously expensive. This used to perform

[Richard Smith via cfe-commits]

On Wed, May 18, 2016 at 6:49 PM, Sean Silva via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Wed, May 18, 2016 at 6:39 PM, Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Wed May 18 20:39:10 2016
>> New Revision: 270009
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=270009=rev
>> Log:
>> Make Sema::getPrintingPolicy less ridiculously expensive. This used to
>> perform
>> an identifier table lookup, *and* copy the LangOptions (including various
>> std::vectors). Twice. We call this function once each time
>> we start
>> parsing a declaration specifier sequence, and once for each call to
>> Sema::Diag.
>>
>> This reduces the compile time for a sample .c file from the linux kernel
>> by 20%.
>>
>> Modified:
>> cfe/trunk/include/clang/AST/ASTContext.h
>> cfe/trunk/include/clang/AST/PrettyPrinter.h
>> cfe/trunk/lib/AST/DeclarationName.cpp
>> cfe/trunk/lib/AST/StmtPrinter.cpp
>> cfe/trunk/lib/AST/TypePrinter.cpp
>> cfe/trunk/lib/Parse/ParseDecl.cpp
>> cfe/trunk/lib/Sema/Sema.cpp
>> cfe/trunk/lib/Sema/SemaCodeComplete.cpp
>> cfe/trunk/test/Analysis/initializers-cfg-output.cpp
>> cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp
>> cfe/trunk/test/SemaCXX/member-pointer.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/ASTContext.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=270009=270008=270009=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/ASTContext.h (original)
>> +++ cfe/trunk/include/clang/AST/ASTContext.h Wed May 18 20:39:10 2016
>> @@ -243,6 +243,9 @@ class ASTContext : public RefCountedBase
>>QualType ObjCClassRedefinitionType;
>>QualType ObjCSelRedefinitionType;
>>
>> +  /// The identifier 'bool'.
>> +  mutable IdentifierInfo *BoolName = nullptr;
>> +
>>/// The identifier 'NSObject'.
>>IdentifierInfo *NSObjectName = nullptr;
>>
>> @@ -1457,6 +1460,13 @@ public:
>>  return NSCopyingName;
>>}
>>
>> +  /// Retrieve the identifier 'bool'.
>> +  IdentifierInfo *getBoolName() const {
>> +if (!BoolName)
>> +  BoolName = ("bool");
>> +return BoolName;
>> +  }
>> +
>>IdentifierInfo *getMakeIntegerSeqName() const {
>>  if (!MakeIntegerSeqName)
>>MakeIntegerSeqName = ("__make_integer_seq");
>>
>> Modified: cfe/trunk/include/clang/AST/PrettyPrinter.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/PrettyPrinter.h?rev=270009=270008=270009=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/PrettyPrinter.h (original)
>> +++ cfe/trunk/include/clang/AST/PrettyPrinter.h Wed May 18 20:39:10 2016
>> @@ -32,22 +32,35 @@ public:
>>
>>  /// \brief Describes how types, statements, expressions, and
>>  /// declarations should be printed.
>> +///
>> +/// This type is intended to be small and suitable for passing by value.
>> +/// It is very frequently copied.
>>  struct PrintingPolicy {
>> -  /// \brief Create a default printing policy for C.
>> +  /// \brief Create a default printing policy for the specified language.
>>PrintingPolicy(const LangOptions )
>> -: LangOpts(LO), Indentation(2), SuppressSpecifiers(false),
>> -  SuppressTagKeyword(false),
>> +: Indentation(2), SuppressSpecifiers(false),
>> +  SuppressTagKeyword(LO.CPlusPlus),
>>IncludeTagDefinition(false), SuppressScope(false),
>>SuppressUnwrittenScope(false), SuppressInitializers(false),
>>ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
>>SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
>>SuppressTemplateArgsInCXXConstructors(false),
>> -  Bool(LO.Bool), TerseOutput(false), PolishForDeclaration(false),
>> +  Bool(LO.Bool), Restrict(LO.C99),
>> +  Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
>> +  UseVoidForZeroParams(!LO.CPlusPlus),
>> +  TerseOutput(false), PolishForDeclaration(false),
>>Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
>>IncludeNewlines(true), MSVCFormatting(false) { }
>>
>> -  /// \brief What language we're printing.
>> -  LangOptions LangOpts;
>> +  /// \brief Adjust this printing policy for cases where it's known that
>> +  /// we're printing C++ code (for instance, if AST dumping reaches a
>> +  /// C++-only construct). This should not be used if a real LangOptions
>> +  /// object is available.
>> +  void adjustForCPlusPlus() {
>> +SuppressTagKeyword = true;
>> +Bool = true;
>> +UseVoidForZeroParams = false;
>> +  }
>>
>>/// \brief The number of spaces to use to indent each line.
>>unsigned Indentation : 8;
>> @@ -143,10 +156,23 @@ struct PrintingPolicy {
>>/// constructors.
>>unsigned SuppressTemplateArgsInCXXConstructors : 1;
>>
>> -  /// \brief Whether we can use 'bool' rather than '_Bool', even 

Re: r270009 - Make Sema::getPrintingPolicy less ridiculously expensive. This used to perform

[Sean Silva via cfe-commits]

Ah, just saw 270010

On Wed, May 18, 2016 at 6:49 PM, Sean Silva  wrote:

>
>
> On Wed, May 18, 2016 at 6:39 PM, Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Wed May 18 20:39:10 2016
>> New Revision: 270009
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=270009=rev
>> Log:
>> Make Sema::getPrintingPolicy less ridiculously expensive. This used to
>> perform
>> an identifier table lookup, *and* copy the LangOptions (including various
>> std::vectors). Twice. We call this function once each time
>> we start
>> parsing a declaration specifier sequence, and once for each call to
>> Sema::Diag.
>>
>> This reduces the compile time for a sample .c file from the linux kernel
>> by 20%.
>>
>> Modified:
>> cfe/trunk/include/clang/AST/ASTContext.h
>> cfe/trunk/include/clang/AST/PrettyPrinter.h
>> cfe/trunk/lib/AST/DeclarationName.cpp
>> cfe/trunk/lib/AST/StmtPrinter.cpp
>> cfe/trunk/lib/AST/TypePrinter.cpp
>> cfe/trunk/lib/Parse/ParseDecl.cpp
>> cfe/trunk/lib/Sema/Sema.cpp
>> cfe/trunk/lib/Sema/SemaCodeComplete.cpp
>> cfe/trunk/test/Analysis/initializers-cfg-output.cpp
>> cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp
>> cfe/trunk/test/SemaCXX/member-pointer.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/ASTContext.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=270009=270008=270009=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/ASTContext.h (original)
>> +++ cfe/trunk/include/clang/AST/ASTContext.h Wed May 18 20:39:10 2016
>> @@ -243,6 +243,9 @@ class ASTContext : public RefCountedBase
>>QualType ObjCClassRedefinitionType;
>>QualType ObjCSelRedefinitionType;
>>
>> +  /// The identifier 'bool'.
>> +  mutable IdentifierInfo *BoolName = nullptr;
>> +
>>/// The identifier 'NSObject'.
>>IdentifierInfo *NSObjectName = nullptr;
>>
>> @@ -1457,6 +1460,13 @@ public:
>>  return NSCopyingName;
>>}
>>
>> +  /// Retrieve the identifier 'bool'.
>> +  IdentifierInfo *getBoolName() const {
>> +if (!BoolName)
>> +  BoolName = ("bool");
>> +return BoolName;
>> +  }
>> +
>>IdentifierInfo *getMakeIntegerSeqName() const {
>>  if (!MakeIntegerSeqName)
>>MakeIntegerSeqName = ("__make_integer_seq");
>>
>> Modified: cfe/trunk/include/clang/AST/PrettyPrinter.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/PrettyPrinter.h?rev=270009=270008=270009=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/PrettyPrinter.h (original)
>> +++ cfe/trunk/include/clang/AST/PrettyPrinter.h Wed May 18 20:39:10 2016
>> @@ -32,22 +32,35 @@ public:
>>
>>  /// \brief Describes how types, statements, expressions, and
>>  /// declarations should be printed.
>> +///
>> +/// This type is intended to be small and suitable for passing by value.
>> +/// It is very frequently copied.
>>  struct PrintingPolicy {
>> -  /// \brief Create a default printing policy for C.
>> +  /// \brief Create a default printing policy for the specified language.
>>PrintingPolicy(const LangOptions )
>> -: LangOpts(LO), Indentation(2), SuppressSpecifiers(false),
>> -  SuppressTagKeyword(false),
>> +: Indentation(2), SuppressSpecifiers(false),
>> +  SuppressTagKeyword(LO.CPlusPlus),
>>IncludeTagDefinition(false), SuppressScope(false),
>>SuppressUnwrittenScope(false), SuppressInitializers(false),
>>ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
>>SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
>>SuppressTemplateArgsInCXXConstructors(false),
>> -  Bool(LO.Bool), TerseOutput(false), PolishForDeclaration(false),
>> +  Bool(LO.Bool), Restrict(LO.C99),
>> +  Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
>> +  UseVoidForZeroParams(!LO.CPlusPlus),
>> +  TerseOutput(false), PolishForDeclaration(false),
>>Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
>>IncludeNewlines(true), MSVCFormatting(false) { }
>>
>> -  /// \brief What language we're printing.
>> -  LangOptions LangOpts;
>> +  /// \brief Adjust this printing policy for cases where it's known that
>> +  /// we're printing C++ code (for instance, if AST dumping reaches a
>> +  /// C++-only construct). This should not be used if a real LangOptions
>> +  /// object is available.
>> +  void adjustForCPlusPlus() {
>> +SuppressTagKeyword = true;
>> +Bool = true;
>> +UseVoidForZeroParams = false;
>> +  }
>>
>>/// \brief The number of spaces to use to indent each line.
>>unsigned Indentation : 8;
>> @@ -143,10 +156,23 @@ struct PrintingPolicy {
>>/// constructors.
>>unsigned SuppressTemplateArgsInCXXConstructors : 1;
>>
>> -  /// \brief Whether we can use 'bool' rather than '_Bool', 

Re: r270009 - Make Sema::getPrintingPolicy less ridiculously expensive. This used to perform

[Sean Silva via cfe-commits]

On Wed, May 18, 2016 at 6:39 PM, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Wed May 18 20:39:10 2016
> New Revision: 270009
>
> URL: http://llvm.org/viewvc/llvm-project?rev=270009=rev
> Log:
> Make Sema::getPrintingPolicy less ridiculously expensive. This used to
> perform
> an identifier table lookup, *and* copy the LangOptions (including various
> std::vectors). Twice. We call this function once each time we
> start
> parsing a declaration specifier sequence, and once for each call to
> Sema::Diag.
>
> This reduces the compile time for a sample .c file from the linux kernel
> by 20%.
>
> Modified:
> cfe/trunk/include/clang/AST/ASTContext.h
> cfe/trunk/include/clang/AST/PrettyPrinter.h
> cfe/trunk/lib/AST/DeclarationName.cpp
> cfe/trunk/lib/AST/StmtPrinter.cpp
> cfe/trunk/lib/AST/TypePrinter.cpp
> cfe/trunk/lib/Parse/ParseDecl.cpp
> cfe/trunk/lib/Sema/Sema.cpp
> cfe/trunk/lib/Sema/SemaCodeComplete.cpp
> cfe/trunk/test/Analysis/initializers-cfg-output.cpp
> cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp
> cfe/trunk/test/SemaCXX/member-pointer.cpp
>
> Modified: cfe/trunk/include/clang/AST/ASTContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=270009=270008=270009=diff
>
> ==
> --- cfe/trunk/include/clang/AST/ASTContext.h (original)
> +++ cfe/trunk/include/clang/AST/ASTContext.h Wed May 18 20:39:10 2016
> @@ -243,6 +243,9 @@ class ASTContext : public RefCountedBase
>QualType ObjCClassRedefinitionType;
>QualType ObjCSelRedefinitionType;
>
> +  /// The identifier 'bool'.
> +  mutable IdentifierInfo *BoolName = nullptr;
> +
>/// The identifier 'NSObject'.
>IdentifierInfo *NSObjectName = nullptr;
>
> @@ -1457,6 +1460,13 @@ public:
>  return NSCopyingName;
>}
>
> +  /// Retrieve the identifier 'bool'.
> +  IdentifierInfo *getBoolName() const {
> +if (!BoolName)
> +  BoolName = ("bool");
> +return BoolName;
> +  }
> +
>IdentifierInfo *getMakeIntegerSeqName() const {
>  if (!MakeIntegerSeqName)
>MakeIntegerSeqName = ("__make_integer_seq");
>
> Modified: cfe/trunk/include/clang/AST/PrettyPrinter.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/PrettyPrinter.h?rev=270009=270008=270009=diff
>
> ==
> --- cfe/trunk/include/clang/AST/PrettyPrinter.h (original)
> +++ cfe/trunk/include/clang/AST/PrettyPrinter.h Wed May 18 20:39:10 2016
> @@ -32,22 +32,35 @@ public:
>
>  /// \brief Describes how types, statements, expressions, and
>  /// declarations should be printed.
> +///
> +/// This type is intended to be small and suitable for passing by value.
> +/// It is very frequently copied.
>  struct PrintingPolicy {
> -  /// \brief Create a default printing policy for C.
> +  /// \brief Create a default printing policy for the specified language.
>PrintingPolicy(const LangOptions )
> -: LangOpts(LO), Indentation(2), SuppressSpecifiers(false),
> -  SuppressTagKeyword(false),
> +: Indentation(2), SuppressSpecifiers(false),
> +  SuppressTagKeyword(LO.CPlusPlus),
>IncludeTagDefinition(false), SuppressScope(false),
>SuppressUnwrittenScope(false), SuppressInitializers(false),
>ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
>SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
>SuppressTemplateArgsInCXXConstructors(false),
> -  Bool(LO.Bool), TerseOutput(false), PolishForDeclaration(false),
> +  Bool(LO.Bool), Restrict(LO.C99),
> +  Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
> +  UseVoidForZeroParams(!LO.CPlusPlus),
> +  TerseOutput(false), PolishForDeclaration(false),
>Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
>IncludeNewlines(true), MSVCFormatting(false) { }
>
> -  /// \brief What language we're printing.
> -  LangOptions LangOpts;
> +  /// \brief Adjust this printing policy for cases where it's known that
> +  /// we're printing C++ code (for instance, if AST dumping reaches a
> +  /// C++-only construct). This should not be used if a real LangOptions
> +  /// object is available.
> +  void adjustForCPlusPlus() {
> +SuppressTagKeyword = true;
> +Bool = true;
> +UseVoidForZeroParams = false;
> +  }
>
>/// \brief The number of spaces to use to indent each line.
>unsigned Indentation : 8;
> @@ -143,10 +156,23 @@ struct PrintingPolicy {
>/// constructors.
>unsigned SuppressTemplateArgsInCXXConstructors : 1;
>
> -  /// \brief Whether we can use 'bool' rather than '_Bool', even if the
> language
> -  /// doesn't actually have 'bool' (because, e.g., it is defined as a
> macro).
> +  /// \brief Whether we can use 'bool' rather than '_Bool' (even if the
> language
> +  /// doesn't actually have