Solved here: http://llvm-reviews.chandlerc.com/D31 Waiting for a review.
On Wed, Aug 29, 2012 at 2:37 PM, Timur Iskhodzhanov <[email protected]>wrote: > Hi Alexander, > > I get the following failure on my private bot running AddressSanitizer > tests: > > Assertion failed: MI && "MacroInfo should be non-zero!", file > ..\..\..\..\..\llvm\tools\clang\lib\Lex\PPMacroExpansion.cpp, line 52 > > Is this expected? > > -- > Timur > > On Wed, Aug 29, 2012 at 4:20 AM, Alexander Kornienko <[email protected]> > wrote: > > Author: alexfh > > Date: Tue Aug 28 19:20:03 2012 > > New Revision: 162810 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=162810&view=rev > > Log: > > Keep history of macro definitions and #undefs > > > > Summary: > > Summary: Keep history of macro definitions and #undefs with > corresponding source locations, so that we can later find out all macros > active in a specified source location. We don't save the history in PCH (no > need currently). Memory overhead is about sizeof(void*)*3*<number of macro > definitions and #undefs>+<in-memory size of all #undef'd macros> > > > > I've run a test on a file composed of 109 .h files from boost 1.49 on > x86-64 linux. > > Stats before this patch: > > *** Preprocessor Stats: > > 73222 directives found: > > 19171 #define. > > 4345 #undef. > > #include/#include_next/#import: > > 5233 source files entered. > > 27 max include stack depth > > 19210 #if/#ifndef/#ifdef. > > 2384 #else/#elif. > > 6891 #endif. > > 408 #pragma. > > 14466 #if/#ifndef#ifdef regions skipped > > 80023/451669/1270 obj/fn/builtin macros expanded, 85724 on the fast path. > > 127145 token paste (##) operations performed, 11008 on the fast path. > > > > Preprocessor Memory: 5874615B total > > BumpPtr: 4399104 > > Macro Expanded Tokens: 417768 > > Predefines Buffer: 8135 > > Macros: 1048576 > > #pragma push_macro Info: 0 > > Poison Reasons: 1024 > > Comment Handlers: 8 > > > > Stats with this patch: > > ... > > Preprocessor Memory: 7541687B total > > BumpPtr: 6066176 > > Macro Expanded Tokens: 417768 > > Predefines Buffer: 8135 > > Macros: 1048576 > > #pragma push_macro Info: 0 > > Poison Reasons: 1024 > > Comment Handlers: 8 > > > > In my test increase in memory usage is about 1.7Mb, which is ~28% of > initial preprocessor's memory usage and about 0.8% of clang's total VMM > allocation. > > > > As for CPU overhead, it should only be noticeable when iterating over > all macros, and should mostly consist of couple extra dereferences and one > comparison per macro + skipping of #undef'd macros. It's less trivial to > measure, though, as the preprocessor consumes a very small fraction of > compilation time. > > > > > > Reviewers: doug.gregor, klimek, rsmith, djasper > > > > Reviewed By: doug.gregor > > > > CC: cfe-commits, chandlerc > > > > Differential Revision: http://llvm-reviews.chandlerc.com/D28 > > > > Modified: > > cfe/trunk/include/clang/Lex/MacroInfo.h > > cfe/trunk/include/clang/Lex/Preprocessor.h > > cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp > > cfe/trunk/lib/Lex/MacroInfo.cpp > > cfe/trunk/lib/Lex/PPDirectives.cpp > > cfe/trunk/lib/Lex/PPMacroExpansion.cpp > > cfe/trunk/lib/Lex/Pragma.cpp > > cfe/trunk/lib/Sema/SemaCodeComplete.cpp > > cfe/trunk/lib/Serialization/ASTWriter.cpp > > > > Modified: cfe/trunk/include/clang/Lex/MacroInfo.h > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/MacroInfo.h?rev=162810&r1=162809&r2=162810&view=diff > > > ============================================================================== > > --- cfe/trunk/include/clang/Lex/MacroInfo.h (original) > > +++ cfe/trunk/include/clang/Lex/MacroInfo.h Tue Aug 28 19:20:03 2012 > > @@ -32,6 +32,12 @@ > > SourceLocation Location; > > /// EndLocation - The location of the last token in the macro. > > SourceLocation EndLocation; > > + /// \brief The location where the macro was #undef'd, or an invalid > location > > + /// for macros that haven't been undefined. > > + SourceLocation UndefLocation; > > + /// \brief Previous definition, the identifier of this macro was > defined to, > > + /// or NULL. > > + MacroInfo *PreviousDefinition; > > > > /// Arguments - The list of arguments for a function-like macro. > This can be > > /// empty, for, e.g. "#define X()". In a C99-style variadic macro, > this > > @@ -128,10 +134,31 @@ > > /// setDefinitionEndLoc - Set the location of the last token in the > macro. > > /// > > void setDefinitionEndLoc(SourceLocation EndLoc) { EndLocation = > EndLoc; } > > + > > /// getDefinitionEndLoc - Return the location of the last token in > the macro. > > /// > > SourceLocation getDefinitionEndLoc() const { return EndLocation; } > > - > > + > > + /// \brief Set the location where macro was undefined. Can only be > set once. > > + void setUndefLoc(SourceLocation UndefLoc) { > > + assert(UndefLocation.isInvalid() && "UndefLocation is already > set!"); > > + assert(UndefLoc.isValid() && "Invalid UndefLoc!"); > > + UndefLocation = UndefLoc; > > + } > > + > > + /// \brief Get the location where macro was undefined. > > + SourceLocation getUndefLoc() const { return UndefLocation; } > > + > > + /// \brief Set previous definition of the macro with the same name. > Can only > > + /// be set once. > > + void setPreviousDefinition(MacroInfo *PreviousDef) { > > + assert(!PreviousDefinition && "PreviousDefiniton is already set!"); > > + PreviousDefinition = PreviousDef; > > + } > > + > > + /// \brief Get previous definition of the macro with the same name. > > + MacroInfo *getPreviousDefinition() { return PreviousDefinition; } > > + > > /// \brief Get length in characters of the macro definition. > > unsigned getDefinitionLength(SourceManager &SM) const { > > if (IsDefinitionLengthCached) > > > > Modified: cfe/trunk/include/clang/Lex/Preprocessor.h > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=162810&r1=162809&r2=162810&view=diff > > > ============================================================================== > > --- cfe/trunk/include/clang/Lex/Preprocessor.h (original) > > +++ cfe/trunk/include/clang/Lex/Preprocessor.h Tue Aug 28 19:20:03 2012 > > @@ -274,8 +274,9 @@ > > }; > > SmallVector<MacroExpandsInfo, 2> DelayedMacroExpandsCallbacks; > > > > - /// Macros - For each IdentifierInfo with 'HasMacro' set, we keep a > mapping > > - /// to the actual definition of the macro. > > + /// Macros - For each IdentifierInfo that was associated with a > macro, we > > + /// keep a mapping to the history of all macro definitions and > #undefs in > > + /// the reverse order (the latest one is in the head of the list). > > llvm::DenseMap<IdentifierInfo*, MacroInfo*> Macros; > > > > /// \brief Macros that we want to warn because they are not used at > the end > > @@ -470,8 +471,10 @@ > > void setMacroInfo(IdentifierInfo *II, MacroInfo *MI, > > bool LoadedFromAST = false); > > > > - /// macro_iterator/macro_begin/macro_end - This allows you to walk > the current > > - /// state of the macro table. This visits every currently-defined > macro. > > + /// macro_iterator/macro_begin/macro_end - This allows you to walk > the macro > > + /// history table. Currently defined macros have > > + /// IdentifierInfo::hasMacroDefinition() set and an empty > > + /// MacroInfo::getUndefLoc() at the head of the list. > > typedef llvm::DenseMap<IdentifierInfo*, > > MacroInfo*>::const_iterator macro_iterator; > > macro_iterator macro_begin(bool IncludeExternalMacros = true) const; > > > > Modified: cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp?rev=162810&r1=162809&r2=162810&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp (original) > > +++ cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp Tue Aug 28 > 19:20:03 2012 > > @@ -570,8 +570,12 @@ > > do PP.Lex(Tok); > > while (Tok.isNot(tok::eof)); > > > > - SmallVector<id_macro_pair, 128> > > - MacrosByID(PP.macro_begin(), PP.macro_end()); > > + SmallVector<id_macro_pair, 128> MacrosByID; > > + for (Preprocessor::macro_iterator I = PP.macro_begin(), E = > PP.macro_end(); > > + I != E; ++I) { > > + if (I->first->hasMacroDefinition()) > > + MacrosByID.push_back(id_macro_pair(I->first, I->second)); > > + } > > llvm::array_pod_sort(MacrosByID.begin(), MacrosByID.end(), > MacroIDCompare); > > > > for (unsigned i = 0, e = MacrosByID.size(); i != e; ++i) { > > > > Modified: cfe/trunk/lib/Lex/MacroInfo.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/MacroInfo.cpp?rev=162810&r1=162809&r2=162810&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/Lex/MacroInfo.cpp (original) > > +++ cfe/trunk/lib/Lex/MacroInfo.cpp Tue Aug 28 19:20:03 2012 > > @@ -15,44 +15,46 @@ > > #include "clang/Lex/Preprocessor.h" > > using namespace clang; > > > > -MacroInfo::MacroInfo(SourceLocation DefLoc) : Location(DefLoc) { > > - IsFunctionLike = false; > > - IsC99Varargs = false; > > - IsGNUVarargs = false; > > - IsBuiltinMacro = false; > > - IsFromAST = false; > > - ChangedAfterLoad = false; > > - IsDisabled = false; > > - IsUsed = false; > > - IsAllowRedefinitionsWithoutWarning = false; > > - IsWarnIfUnused = false; > > - IsDefinitionLengthCached = false; > > - IsPublic = true; > > - > > - ArgumentList = 0; > > - NumArguments = 0; > > +MacroInfo::MacroInfo(SourceLocation DefLoc) > > + : Location(DefLoc), > > + PreviousDefinition(0), > > + ArgumentList(0), > > + NumArguments(0), > > + IsDefinitionLengthCached(false), > > + IsFunctionLike(false), > > + IsC99Varargs(false), > > + IsGNUVarargs(false), > > + IsBuiltinMacro(false), > > + IsFromAST(false), > > + ChangedAfterLoad(false), > > + IsDisabled(false), > > + IsUsed(false), > > + IsAllowRedefinitionsWithoutWarning(false), > > + IsWarnIfUnused(false), > > + IsPublic(true) { > > } > > > > -MacroInfo::MacroInfo(const MacroInfo &MI, llvm::BumpPtrAllocator > &PPAllocator) { > > - Location = MI.Location; > > - EndLocation = MI.EndLocation; > > - ReplacementTokens = MI.ReplacementTokens; > > - IsFunctionLike = MI.IsFunctionLike; > > - IsC99Varargs = MI.IsC99Varargs; > > - IsGNUVarargs = MI.IsGNUVarargs; > > - IsBuiltinMacro = MI.IsBuiltinMacro; > > - IsFromAST = MI.IsFromAST; > > - ChangedAfterLoad = MI.ChangedAfterLoad; > > - IsDisabled = MI.IsDisabled; > > - IsUsed = MI.IsUsed; > > - IsAllowRedefinitionsWithoutWarning = > MI.IsAllowRedefinitionsWithoutWarning; > > - IsWarnIfUnused = MI.IsWarnIfUnused; > > - IsDefinitionLengthCached = MI.IsDefinitionLengthCached; > > - DefinitionLength = MI.DefinitionLength; > > - IsPublic = MI.IsPublic; > > - > > - ArgumentList = 0; > > - NumArguments = 0; > > +MacroInfo::MacroInfo(const MacroInfo &MI, llvm::BumpPtrAllocator > &PPAllocator) > > + : Location(MI.Location), > > + EndLocation(MI.EndLocation), > > + UndefLocation(MI.UndefLocation), > > + PreviousDefinition(0), > > + ArgumentList(0), > > + NumArguments(0), > > + ReplacementTokens(MI.ReplacementTokens), > > + DefinitionLength(MI.DefinitionLength), > > + IsDefinitionLengthCached(MI.IsDefinitionLengthCached), > > + IsFunctionLike(MI.IsFunctionLike), > > + IsC99Varargs(MI.IsC99Varargs), > > + IsGNUVarargs(MI.IsGNUVarargs), > > + IsBuiltinMacro(MI.IsBuiltinMacro), > > + IsFromAST(MI.IsFromAST), > > + ChangedAfterLoad(MI.ChangedAfterLoad), > > + IsDisabled(MI.IsDisabled), > > + IsUsed(MI.IsUsed), > > + > IsAllowRedefinitionsWithoutWarning(MI.IsAllowRedefinitionsWithoutWarning), > > + IsWarnIfUnused(MI.IsWarnIfUnused), > > + IsPublic(MI.IsPublic) { > > setArgumentList(MI.ArgumentList, MI.NumArguments, PPAllocator); > > } > > > > > > Modified: cfe/trunk/lib/Lex/PPDirectives.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=162810&r1=162809&r2=162810&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/Lex/PPDirectives.cpp (original) > > +++ cfe/trunk/lib/Lex/PPDirectives.cpp Tue Aug 28 19:20:03 2012 > > @@ -1849,7 +1849,7 @@ > > MI->setDefinitionEndLoc(LastTok.getLocation()); > > > > // Finally, if this identifier already had a macro defined for it, > verify that > > - // the macro bodies are identical and free the old definition. > > + // the macro bodies are identical, and issue diagnostics if they are > not. > > if (MacroInfo *OtherMI = > getMacroInfo(MacroNameTok.getIdentifierInfo())) { > > // It is very common for system headers to have tons of macro > redefinitions > > // and for warnings to be disabled in system headers. If this is > the case, > > @@ -1870,7 +1870,6 @@ > > } > > if (OtherMI->isWarnIfUnused()) > > WarnUnusedMacroLocs.erase(OtherMI->getDefinitionLoc()); > > - ReleaseMacroInfo(OtherMI); > > } > > > > setMacroInfo(MacroNameTok.getIdentifierInfo(), MI); > > @@ -1921,9 +1920,11 @@ > > if (MI->isWarnIfUnused()) > > WarnUnusedMacroLocs.erase(MI->getDefinitionLoc()); > > > > - // Free macro definition. > > - ReleaseMacroInfo(MI); > > - setMacroInfo(MacroNameTok.getIdentifierInfo(), 0); > > + MI->setUndefLoc(MacroNameTok.getLocation()); > > + IdentifierInfo *II = MacroNameTok.getIdentifierInfo(); > > + II->setHasMacroDefinition(false); > > + if (II->isFromAST()) > > + II->setChangedSinceDeserialization(); > > } > > > > > > > > Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=162810&r1=162809&r2=162810&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original) > > +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Tue Aug 28 19:20:03 2012 > > @@ -33,15 +33,15 @@ > > > > MacroInfo *Preprocessor::getInfoForMacro(IdentifierInfo *II) const { > > assert(II->hasMacroDefinition() && "Identifier is not a macro!"); > > - > > - llvm::DenseMap<IdentifierInfo*, MacroInfo*>::const_iterator Pos > > - = Macros.find(II); > > + > > + macro_iterator Pos = Macros.find(II); > > if (Pos == Macros.end()) { > > // Load this macro from the external source. > > getExternalSource()->LoadMacroDefinition(II); > > Pos = Macros.find(II); > > } > > assert(Pos != Macros.end() && "Identifier macro info is missing!"); > > + assert(Pos->second->getUndefLoc().isInvalid() && "Macro is > undefined!"); > > return Pos->second; > > } > > > > @@ -49,17 +49,12 @@ > > /// > > void Preprocessor::setMacroInfo(IdentifierInfo *II, MacroInfo *MI, > > bool LoadedFromAST) { > > - if (MI) { > > - Macros[II] = MI; > > - II->setHasMacroDefinition(true); > > - if (II->isFromAST() && !LoadedFromAST) > > - II->setChangedSinceDeserialization(); > > - } else if (II->hasMacroDefinition()) { > > - Macros.erase(II); > > - II->setHasMacroDefinition(false); > > - if (II->isFromAST() && !LoadedFromAST) > > - II->setChangedSinceDeserialization(); > > - } > > + assert(MI && "MacroInfo should be non-zero!"); > > + MI->setPreviousDefinition(Macros[II]); > > + Macros[II] = MI; > > + II->setHasMacroDefinition(true); > > + if (II->isFromAST() && !LoadedFromAST) > > + II->setChangedSinceDeserialization(); > > } > > > > /// RegisterBuiltinMacro - Register the specified identifier in the > identifier > > > > Modified: cfe/trunk/lib/Lex/Pragma.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Pragma.cpp?rev=162810&r1=162809&r2=162810&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/Lex/Pragma.cpp (original) > > +++ cfe/trunk/lib/Lex/Pragma.cpp Tue Aug 28 19:20:03 2012 > > @@ -733,12 +733,10 @@ > > llvm::DenseMap<IdentifierInfo*, std::vector<MacroInfo*> >::iterator > iter = > > PragmaPushMacroInfo.find(IdentInfo); > > if (iter != PragmaPushMacroInfo.end()) { > > - // Release the MacroInfo currently associated with IdentInfo. > > - MacroInfo *CurrentMI = getMacroInfo(IdentInfo); > > - if (CurrentMI) { > > + // Forget the MacroInfo currently associated with IdentInfo. > > + if (MacroInfo *CurrentMI = getMacroInfo(IdentInfo)) { > > if (CurrentMI->isWarnIfUnused()) > > WarnUnusedMacroLocs.erase(CurrentMI->getDefinitionLoc()); > > - ReleaseMacroInfo(CurrentMI); > > } > > > > // Get the MacroInfo we want to reinstall. > > > > Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=162810&r1=162809&r2=162810&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original) > > +++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Tue Aug 28 19:20:03 2012 > > @@ -2904,7 +2904,11 @@ > > for (Preprocessor::macro_iterator M = PP.macro_begin(), > > MEnd = PP.macro_end(); > > M != MEnd; ++M) { > > - Results.AddResult(Result(M->first, > > + // FIXME: Eventually, we'd want to be able to look back to the macro > > + // definition that was actually active at the point of code > completion (even > > + // if that macro has since been #undef'd). > > + if (M->first->hasMacroDefinition()) > > + Results.AddResult(Result(M->first, > > getMacroUsagePriority(M->first->getName(), > > PP.getLangOpts(), > > > TargetTypeIsPointer))); > > > > Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=162810&r1=162809&r2=162810&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original) > > +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Tue Aug 28 19:20:03 2012 > > @@ -1677,10 +1677,12 @@ > > for (Preprocessor::macro_iterator I = PP.macro_begin(Chain == 0), > > E = PP.macro_end(Chain == 0); > > I != E; ++I) { > > - const IdentifierInfo *Name = I->first; > > - if (!IsModule || I->second->isPublic()) { > > - MacroDefinitionsSeen.insert(Name); > > - MacrosToEmit.push_back(std::make_pair(I->first, I->second)); > > + // FIXME: We'll need to store macro history in PCH. > > + if (I->first->hasMacroDefinition()) { > > + if (!IsModule || I->second->isPublic()) { > > + MacroDefinitionsSeen.insert(I->first); > > + MacrosToEmit.push_back(std::make_pair(I->first, I->second)); > > + } > > } > > } > > > > > > > > _______________________________________________ > > cfe-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > -- Alexander Kornienko | Software Engineer | [email protected] | +49 151 221 77 957 Google Germany GmbH | Dienerstr. 12 | 80331 München
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
