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

Sean Silva via cfe-commits cfe-commits at lists.llvm.org
Wed May 18 18:50:38 PDT 2016


Ah, just saw 270010

On Wed, May 18, 2016 at 6:49 PM, Sean Silva <chisophugis at gmail.com> wrote:

>
>
> On Wed, May 18, 2016 at 6:39 PM, Richard Smith via cfe-commits <
> cfe-commits at 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&view=rev
>> Log:
>> Make Sema::getPrintingPolicy less ridiculously expensive. This used to
>> perform
>> an identifier table lookup, *and* copy the LangOptions (including various
>> std::vector<std::string>s). 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&r1=270008&r2=270009&view=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 = &Idents.get("bool");
>> +    return BoolName;
>> +  }
>> +
>>    IdentifierInfo *getMakeIntegerSeqName() const {
>>      if (!MakeIntegerSeqName)
>>        MakeIntegerSeqName = &Idents.get("__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&r1=270008&r2=270009&view=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 &LO)
>> -    : 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 'bool', because, e.g., it is defined as a
>> macro).
>>    unsigned Bool : 1;
>>
>> +  /// \brief Whether we can use 'restrict' rather than '__restrict'.
>> +  unsigned Restrict : 1;
>> +
>> +  /// \brief Whether we can use 'alignof' rather than '__alignof'.
>> +  unsigned Alignof : 1;
>> +
>> +  /// \brief Whether we can use '_Alignof' rather than '__alignof'.
>> +  unsigned UnderscoreAlignof : 1;
>> +
>> +  /// \brief Whether we should use '(void)' rather than '()' for a
>> function
>> +  /// prototype with zero parameters.
>> +  unsigned UseVoidForZeroParams : 1;
>> +
>>    /// \brief Provide a 'terse' output.
>>    ///
>>    /// For example, in this mode we don't print function bodies, class
>> members,
>>
>> Modified: cfe/trunk/lib/AST/DeclarationName.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclarationName.cpp?rev=270009&r1=270008&r2=270009&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/AST/DeclarationName.cpp (original)
>> +++ cfe/trunk/lib/AST/DeclarationName.cpp Wed May 18 20:39:10 2016
>> @@ -135,7 +135,10 @@ int DeclarationName::compare(Declaration
>>
>>  static void printCXXConstructorDestructorName(QualType ClassType,
>>                                                raw_ostream &OS,
>> -                                              const PrintingPolicy
>> &Policy) {
>> +                                              PrintingPolicy Policy) {
>> +  // We know we're printing C++ here. Ensure we print types properly.
>> +  Policy.adjustForCPlusPlus();
>> +
>>    if (const RecordType *ClassRec = ClassType->getAs<RecordType>()) {
>>      OS << *ClassRec->getDecl();
>>      return;
>> @@ -146,14 +149,7 @@ static void printCXXConstructorDestructo
>>        return;
>>      }
>>    }
>> -  if (!Policy.LangOpts.CPlusPlus) {
>> -    // Passed policy is the default one from operator <<, use a C++
>> policy.
>> -    LangOptions LO;
>> -    LO.CPlusPlus = true;
>> -    ClassType.print(OS, PrintingPolicy(LO));
>> -  } else {
>> -    ClassType.print(OS, Policy);
>> -  }
>> +  ClassType.print(OS, Policy);
>>  }
>>
>>  void DeclarationName::print(raw_ostream &OS, const PrintingPolicy
>> &Policy) {
>> @@ -206,15 +202,10 @@ void DeclarationName::print(raw_ostream
>>        OS << *Rec->getDecl();
>>        return;
>>      }
>> -    if (!Policy.LangOpts.CPlusPlus) {
>> -      // Passed policy is the default one from operator <<, use a C++
>> policy.
>> -      LangOptions LO;
>> -      LO.CPlusPlus = true;
>> -      LO.Bool = true;
>> -      Type.print(OS, PrintingPolicy(LO));
>> -    } else {
>> -      Type.print(OS, Policy);
>> -    }
>> +    // We know we're printing C++ here, ensure we print 'bool' properly.
>> +    PrintingPolicy CXXPolicy = Policy;
>> +    CXXPolicy.adjustForCPlusPlus();
>> +    Type.print(OS, CXXPolicy);
>>      return;
>>    }
>>    case DeclarationName::CXXUsingDirective:
>>
>> Modified: cfe/trunk/lib/AST/StmtPrinter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/StmtPrinter.cpp?rev=270009&r1=270008&r2=270009&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/AST/StmtPrinter.cpp (original)
>> +++ cfe/trunk/lib/AST/StmtPrinter.cpp Wed May 18 20:39:10 2016
>> @@ -1397,9 +1397,9 @@ void StmtPrinter::VisitUnaryExprOrTypeTr
>>      OS << "sizeof";
>>      break;
>>    case UETT_AlignOf:
>> -    if (Policy.LangOpts.CPlusPlus)
>> +    if (Policy.Alignof)
>>        OS << "alignof";
>> -    else if (Policy.LangOpts.C11)
>> +    else if (Policy.UnderscoreAlignof)
>>        OS << "_Alignof";
>>      else
>>        OS << "__alignof";
>> @@ -1669,7 +1669,7 @@ void StmtPrinter::VisitNoInitExpr(NoInit
>>  }
>>
>>  void StmtPrinter::VisitImplicitValueInitExpr(ImplicitValueInitExpr
>> *Node) {
>> -  if (Policy.LangOpts.CPlusPlus) {
>> +  if (Node->getType()->getAsCXXRecordDecl()) {
>>      OS << "/*implicit*/";
>>      Node->getType().print(OS, Policy);
>>      OS << "()";
>>
>> Modified: cfe/trunk/lib/AST/TypePrinter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TypePrinter.cpp?rev=270009&r1=270008&r2=270009&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/AST/TypePrinter.cpp (original)
>> +++ cfe/trunk/lib/AST/TypePrinter.cpp Wed May 18 20:39:10 2016
>> @@ -112,7 +112,8 @@ namespace {
>>    };
>>  }
>>
>> -static void AppendTypeQualList(raw_ostream &OS, unsigned TypeQuals, bool
>> C99) {
>> +static void AppendTypeQualList(raw_ostream &OS, unsigned TypeQuals,
>> +                               bool HasRestrictKeyword) {
>>    bool appendSpace = false;
>>    if (TypeQuals & Qualifiers::Const) {
>>      OS << "const";
>> @@ -125,7 +126,7 @@ static void AppendTypeQualList(raw_ostre
>>    }
>>    if (TypeQuals & Qualifiers::Restrict) {
>>      if (appendSpace) OS << ' ';
>> -    if (C99) {
>> +    if (HasRestrictKeyword) {
>>        OS << "restrict";
>>      } else {
>>        OS << "__restrict";
>> @@ -439,7 +440,8 @@ void TypePrinter::printConstantArrayAfte
>>                                            raw_ostream &OS) {
>>    OS << '[';
>>    if (T->getIndexTypeQualifiers().hasQualifiers()) {
>> -    AppendTypeQualList(OS, T->getIndexTypeCVRQualifiers(),
>> Policy.LangOpts.C99);
>> +    AppendTypeQualList(OS, T->getIndexTypeCVRQualifiers(),
>> +                       Policy.Restrict);
>>      OS << ' ';
>>    }
>>
>> @@ -472,7 +474,7 @@ void TypePrinter::printVariableArrayAfte
>>                                            raw_ostream &OS) {
>>    OS << '[';
>>    if (T->getIndexTypeQualifiers().hasQualifiers()) {
>> -    AppendTypeQualList(OS, T->getIndexTypeCVRQualifiers(),
>> Policy.LangOpts.C99);
>> +    AppendTypeQualList(OS, T->getIndexTypeCVRQualifiers(),
>> Policy.Restrict);
>>      OS << ' ';
>>    }
>>
>> @@ -672,7 +674,7 @@ void TypePrinter::printFunctionProtoAfte
>>      if (T->getNumParams())
>>        OS << ", ";
>>      OS << "...";
>> -  } else if (T->getNumParams() == 0 && !Policy.LangOpts.CPlusPlus) {
>> +  } else if (T->getNumParams() == 0 && Policy.UseVoidForZeroParams) {
>>      // Do not emit int() if we have a proto, emit 'int(void)'.
>>      OS << "void";
>>    }
>> @@ -746,7 +748,7 @@ void TypePrinter::printFunctionProtoAfte
>>
>>    if (unsigned quals = T->getTypeQuals()) {
>>      OS << ' ';
>> -    AppendTypeQualList(OS, quals, Policy.LangOpts.C99);
>> +    AppendTypeQualList(OS, quals, Policy.Restrict);
>>    }
>>
>>    switch (T->getRefQualifier()) {
>> @@ -947,13 +949,9 @@ void TypePrinter::printTag(TagDecl *D, r
>>
>>    bool HasKindDecoration = false;
>>
>> -  // bool SuppressTagKeyword
>> -  //   = Policy.LangOpts.CPlusPlus || Policy.SuppressTagKeyword;
>> -
>>    // We don't print tags unless this is an elaborated type.
>>    // In C, we just assume every RecordType is an elaborated type.
>> -  if (!(Policy.LangOpts.CPlusPlus || Policy.SuppressTagKeyword ||
>> -        D->getTypedefNameForAnonDecl())) {
>> +  if (!Policy.SuppressTagKeyword && !D->getTypedefNameForAnonDecl()) {
>>      HasKindDecoration = true;
>>      OS << D->getKindName();
>>      OS << ' ';
>> @@ -1590,7 +1588,7 @@ void Qualifiers::print(raw_ostream &OS,
>>
>>    unsigned quals = getCVRQualifiers();
>>    if (quals) {
>> -    AppendTypeQualList(OS, quals, Policy.LangOpts.C99);
>> +    AppendTypeQualList(OS, quals, Policy.Restrict);
>>      addSpace = true;
>>    }
>>    if (hasUnaligned()) {
>>
>> Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=270009&r1=270008&r2=270009&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
>> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Wed May 18 20:39:10 2016
>> @@ -2661,7 +2661,7 @@ void Parser::ParseDeclarationSpecifiers(
>>    bool AttrsLastTime = false;
>>    ParsedAttributesWithRange attrs(AttrFactory);
>>    // We use Sema's policy to get bool macros right.
>> -  const PrintingPolicy &Policy = Actions.getPrintingPolicy();
>> +  PrintingPolicy Policy = Actions.getPrintingPolicy();
>>    while (1) {
>>      bool isInvalid = false;
>>      bool isStorageClass = false;
>>
>> Modified: cfe/trunk/lib/Sema/Sema.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=270009&r1=270008&r2=270009&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/Sema.cpp (original)
>> +++ cfe/trunk/lib/Sema/Sema.cpp Wed May 18 20:39:10 2016
>> @@ -52,13 +52,14 @@ ModuleLoader &Sema::getModuleLoader() co
>>  PrintingPolicy Sema::getPrintingPolicy(const ASTContext &Context,
>>                                         const Preprocessor &PP) {
>>    PrintingPolicy Policy = Context.getPrintingPolicy();
>> +  // Our printing policy is copied over the ASTContext printing policy
>> whenever
>> +  // a diagnostic is emitted, so recompute it.
>>    Policy.Bool = Context.getLangOpts().Bool;
>>    if (!Policy.Bool) {
>> -    if (const MacroInfo *
>> -          BoolMacro = PP.getMacroInfo(&Context.Idents.get("bool"))) {
>> +    if (const MacroInfo *BoolMacro =
>> PP.getMacroInfo(Context.getBoolName())) {
>>        Policy.Bool = BoolMacro->isObjectLike() &&
>> -        BoolMacro->getNumTokens() == 1 &&
>> -        BoolMacro->getReplacementToken(0).is(tok::kw__Bool);
>> +                    BoolMacro->getNumTokens() == 1 &&
>> +                    BoolMacro->getReplacementToken(0).is(tok::kw__Bool);
>>      }
>>    }
>>
>>
>> Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=270009&r1=270008&r2=270009&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Wed May 18 20:39:10 2016
>> @@ -1517,7 +1517,6 @@ static void AddOrdinaryNameResults(Sema:
>>                                     ResultBuilder &Results) {
>>    CodeCompletionAllocator &Allocator = Results.getAllocator();
>>    CodeCompletionBuilder Builder(Allocator,
>> Results.getCodeCompletionTUInfo());
>> -  PrintingPolicy Policy = getCompletionPrintingPolicy(SemaRef);
>>
>>    typedef CodeCompletionResult Result;
>>    switch (CCC) {
>>
>> Modified: cfe/trunk/test/Analysis/initializers-cfg-output.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/initializers-cfg-output.cpp?rev=270009&r1=270008&r2=270009&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/initializers-cfg-output.cpp (original)
>> +++ cfe/trunk/test/Analysis/initializers-cfg-output.cpp Wed May 18
>> 20:39:10 2016
>> @@ -61,7 +61,7 @@ class TestDelegating {
>>  // CHECK:    6: B([B1.5]) (Base initializer)
>>  // CHECK:    7:  (CXXConstructExpr, class A)
>>  // CHECK:    8: A([B1.7]) (Base initializer)
>> -// CHECK:    9: /*implicit*/int()
>> +// CHECK:    9: /*implicit*/(int)0
>>  // CHECK:   10: i([B1.9]) (Member initializer)
>>  // CHECK:   11: this
>>  // CHECK:   12: [B1.11]->i
>>
>> Modified: cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp?rev=270009&r1=270008&r2=270009&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp (original)
>> +++ cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp Wed May 18
>> 20:39:10 2016
>> @@ -1077,7 +1077,7 @@ int testConsistencyNestedNormalReturn(bo
>>  // CHECK:    14: a([B1.13]) (Member initializer)
>>  // CHECK:    15: ~B() (Temporary object destructor)
>>  // CHECK:    16: ~A() (Temporary object destructor)
>> -// CHECK:    17: /*implicit*/int()
>> +// CHECK:    17: /*implicit*/(int)0
>>  // CHECK:    18: b([B1.17]) (Member initializer)
>>  // CHECK:     Preds (1): B2
>>  // CHECK:     Succs (1): B0
>>
>> Modified: cfe/trunk/test/SemaCXX/member-pointer.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/member-pointer.cpp?rev=270009&r1=270008&r2=270009&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/member-pointer.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/member-pointer.cpp Wed May 18 20:39:10 2016
>> @@ -323,3 +323,12 @@ namespace test8 {
>>               .**(int A::**) 0; // expected-warning {{indirection of
>> non-volatile null pointer will be deleted}} expected-note {{consider}}
>>    }
>>  }
>> +
>> +namespace PR27558 {
>> +  template<typename Args> struct A { void f(); };
>> +  template<typename Args> struct B : A<Args> {
>> +    using A<Args>::f;
>> +    B() { (void)&B<Args>::f; }
>> +  };
>> +  B<int> b;
>> +}
>>
>
> Was this PR27558 test case intentional?
>
> -- Sean Silva
>
>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160518/5ca5d8fd/attachment-0001.html>


More information about the cfe-commits mailing list