ogoffart created this revision.
ogoffart added reviewers: cfe-commits, rsmith.
Herald added a subscriber: sanjoy.
DiagnosticIDs::getDiagnosticSeverity function turns out to take a lot of time
in getDecomposedLoc. It is called quite often from different places. (For
example from Sema::CheckTemplateArgument which attempt to emit
warn_cxx98_compat_* diagnostics.)
In most cases, the diagnostics are not overridden by pragmas and the mapping
from the command line is good enough. This commit adds a bit in the the first
mapping which specify if we should look up the real mapping.
This saves more than 5% of the parsing time according to perf.
https://reviews.llvm.org/D26465
Files:
include/clang/Basic/Diagnostic.h
include/clang/Basic/DiagnosticIDs.h
lib/Basic/Diagnostic.cpp
lib/Basic/DiagnosticIDs.cpp
lib/Serialization/ASTReader.cpp
Index: lib/Serialization/ASTReader.cpp
===================================================================
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -5299,6 +5299,10 @@
diag::Severity Map = (diag::Severity)F.PragmaDiagMappings[Idx++];
DiagnosticMapping Mapping = Diag.makeUserMapping(Map, Loc);
Diag.GetCurDiagState()->setMapping(DiagID, Mapping);
+
+ // We are overriding a default behavior
+ Diag.DiagStates.front().getOrAddMapping(DiagID).setMightBeOverriden(
+ true);
}
}
}
Index: lib/Basic/DiagnosticIDs.cpp
===================================================================
--- lib/Basic/DiagnosticIDs.cpp
+++ lib/Basic/DiagnosticIDs.cpp
@@ -406,25 +406,29 @@
DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc,
const DiagnosticsEngine &Diag) const {
assert(getBuiltinDiagClass(DiagID) != CLASS_NOTE);
-
// Specific non-error diagnostics may be mapped to various levels from ignored
// to error. Errors can only be mapped to fatal.
diag::Severity Result = diag::Severity::Fatal;
- DiagnosticsEngine::DiagStatePointsTy::iterator
- Pos = Diag.GetDiagStatePointForLoc(Loc);
- DiagnosticsEngine::DiagState *State = Pos->State;
-
- // Get the mapping information, or compute it lazily.
- DiagnosticMapping &Mapping = State->getOrAddMapping((diag::kind)DiagID);
+ // Calling GetDiagStatePointForLoc might be costly as it needs to create
+ // FullSourceLoc and compare them to the ones in the states. So first query
+ // the base state and check if it might be overridden.
+ DiagnosticsEngine::DiagState &BaseState = Diag.DiagStates.front();
+ assert(&BaseState == Diag.DiagStatePoints.front().State);
+ DiagnosticMapping *Mapping = &BaseState.getOrAddMapping((diag::kind)DiagID);
+ if (Mapping->mightBeOverriden()) {
+ DiagnosticsEngine::DiagStatePointsTy::iterator Pos =
+ Diag.GetDiagStatePointForLoc(Loc);
+ Mapping = &Pos->State->getOrAddMapping((diag::kind)DiagID);
+ }
// TODO: Can a null severity really get here?
- if (Mapping.getSeverity() != diag::Severity())
- Result = Mapping.getSeverity();
+ if (Mapping->getSeverity() != diag::Severity())
+ Result = Mapping->getSeverity();
// Upgrade ignored diagnostics if -Weverything is enabled.
if (Diag.EnableAllWarnings && Result == diag::Severity::Ignored &&
- !Mapping.isUser() && getBuiltinDiagClass(DiagID) != CLASS_REMARK)
+ !Mapping->isUser() && getBuiltinDiagClass(DiagID) != CLASS_REMARK)
Result = diag::Severity::Warning;
// Ignore -pedantic diagnostics inside __extension__ blocks.
@@ -437,7 +441,7 @@
// For extension diagnostics that haven't been explicitly mapped, check if we
// should upgrade the diagnostic.
- if (IsExtensionDiag && !Mapping.isUser())
+ if (IsExtensionDiag && !Mapping->isUser())
Result = std::max(Result, Diag.ExtBehavior);
// At this point, ignored errors can no longer be upgraded.
@@ -451,14 +455,14 @@
// If -Werror is enabled, map warnings to errors unless explicitly disabled.
if (Result == diag::Severity::Warning) {
- if (Diag.WarningsAsErrors && !Mapping.hasNoWarningAsError())
+ if (Diag.WarningsAsErrors && !Mapping->hasNoWarningAsError())
Result = diag::Severity::Error;
}
// If -Wfatal-errors is enabled, map errors to fatal unless explicity
// disabled.
if (Result == diag::Severity::Error) {
- if (Diag.ErrorsAsFatal && !Mapping.hasNoErrorAsFatal())
+ if (Diag.ErrorsAsFatal && !Mapping->hasNoErrorAsFatal())
Result = diag::Severity::Fatal;
}
Index: lib/Basic/Diagnostic.cpp
===================================================================
--- lib/Basic/Diagnostic.cpp
+++ lib/Basic/Diagnostic.cpp
@@ -201,6 +201,11 @@
}
DiagnosticMapping Mapping = makeUserMapping(Map, L);
+ if (Loc.isValid()) {
+ // We are potentially overriding a default behavior
+ DiagStates.front().getOrAddMapping(Diag).setMightBeOverriden(true);
+ }
+
// Common case; setting all the diagnostics of a group in one place.
if (Loc.isInvalid() || Loc == LastStateChangePos) {
GetCurDiagState()->setMapping(Diag, Mapping);
Index: include/clang/Basic/DiagnosticIDs.h
===================================================================
--- include/clang/Basic/DiagnosticIDs.h
+++ include/clang/Basic/DiagnosticIDs.h
@@ -84,6 +84,7 @@
unsigned IsPragma : 1;
unsigned HasNoWarningAsError : 1;
unsigned HasNoErrorAsFatal : 1;
+ unsigned MightBeOverriden : 1;
public:
static DiagnosticMapping Make(diag::Severity Severity, bool IsUser,
@@ -94,6 +95,8 @@
Result.IsPragma = IsPragma;
Result.HasNoWarningAsError = 0;
Result.HasNoErrorAsFatal = 0;
+ Result.MightBeOverriden = 0;
+
return Result;
}
@@ -108,6 +111,11 @@
bool hasNoErrorAsFatal() const { return HasNoErrorAsFatal; }
void setNoErrorAsFatal(bool Value) { HasNoErrorAsFatal = Value; }
+
+ /// Only makes sense for the base state: specify if the diagnostic might be
+ /// changed for other source location
+ bool mightBeOverriden() const { return MightBeOverriden; }
+ void setMightBeOverriden(bool Value) { MightBeOverriden = Value; }
};
/// \brief Used for handling and querying diagnostic IDs.
Index: include/clang/Basic/Diagnostic.h
===================================================================
--- include/clang/Basic/Diagnostic.h
+++ include/clang/Basic/Diagnostic.h
@@ -222,7 +222,10 @@
};
/// \brief Keeps and automatically disposes all DiagStates that we create.
- std::list<DiagState> DiagStates;
+ ///
+ /// The first DiagState represents the state set by the command line. But it
+ /// also contains a cache for the defaults that is lazily populated.
+ mutable std::list<DiagState> DiagStates;
/// \brief Represents a point in source where the diagnostic state was
/// modified because of a pragma.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
