[cfe-commits] r162810 - in /cfe/trunk: include/clang/Lex/MacroInfo.h include/clang/Lex/Preprocessor.h lib/Frontend/PrintPreprocessedOutput.cpp lib/Lex/MacroInfo.cpp lib/Lex/PPDirectives.cpp lib/Lex/PPMacroExpansion.cpp lib/Lex/Pragma.cpp lib/Sema
Timur Iskhodzhanov
timurrrr at google.com
Wed Aug 29 05:37:26 PDT 2012
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 <alexfh at google.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list