On Mar 10, 2011, at 12:03 PM, Ted Kremenek wrote: > Author: kremenek > Date: Thu Mar 10 14:03:42 2011 > New Revision: 127425 > > URL: http://llvm.org/viewvc/llvm-project?rev=127425&view=rev > Log: > Profiling showed that 'CheckImplicitConversions' was very slow because of the > call to getSpellingLoc(). On 'aes.c' > in the LLVM test suite, this function was consuming 7.4% of -fsyntax-only > time. This change fixes this issue > by delaying the check that the warning would be issued within a system macro > by as long as possible. The > main negative of this change is now the logic for this check is done in > multiple places in this function instead > of just in one place up front.
Nice catch Ted. Why are we checking the spelling location in any case? Whether a token came from a system header doesn't sound that interesting to me, and *is* very expensive to compute. -Chris > > Modified: > cfe/trunk/lib/Sema/SemaChecking.cpp > > Modified: cfe/trunk/lib/Sema/SemaChecking.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=127425&r1=127424&r2=127425&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) > +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Mar 10 14:03:42 2011 > @@ -2762,6 +2762,11 @@ > return ValueInRange.toString(10); > } > > +static bool isFromSystemMacro(Sema &S, SourceLocation loc) { > + SourceManager &smgr = S.Context.getSourceManager(); > + return loc.isMacroID() && smgr.isInSystemHeader(smgr.getSpellingLoc(loc)); > +} > + > void CheckImplicitConversion(Sema &S, Expr *E, QualType T, > SourceLocation CC, bool *ICContext = 0) { > if (E->isTypeDependent() || E->isValueDependent()) return; > @@ -2771,11 +2776,12 @@ > if (Source == Target) return; > if (Target->isDependentType()) return; > > - // If the conversion context location is invalid or instantiated > - // from a system macro, don't complain. > - if (CC.isInvalid() || > - (CC.isMacroID() && S.Context.getSourceManager().isInSystemHeader( > - S.Context.getSourceManager().getSpellingLoc(CC)))) > + // If the conversion context location is invalid don't complain. > + // We also don't want to emit a warning if the issue occurs from the > + // instantiation of a system macro. The problem is that 'getSpellingLoc()' > + // is slow, so we delay this check as long as possible. Once we detect > + // we are in that scenario, we just return. > + if (CC.isInvalid()) > return; > > // Never diagnose implicit casts to bool. > @@ -2784,8 +2790,11 @@ > > // Strip vector types. > if (isa<VectorType>(Source)) { > - if (!isa<VectorType>(Target)) > + if (!isa<VectorType>(Target)) { > + if (isFromSystemMacro(S, CC)) > + return; > return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_vector_scalar); > + } > > Source = cast<VectorType>(Source)->getElementType().getTypePtr(); > Target = cast<VectorType>(Target)->getElementType().getTypePtr(); > @@ -2793,8 +2802,12 @@ > > // Strip complex types. > if (isa<ComplexType>(Source)) { > - if (!isa<ComplexType>(Target)) > + if (!isa<ComplexType>(Target)) { > + if (isFromSystemMacro(S, CC)) > + return; > + > return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_complex_scalar); > + } > > Source = cast<ComplexType>(Source)->getElementType().getTypePtr(); > Target = cast<ComplexType>(Target)->getElementType().getTypePtr(); > @@ -2822,13 +2835,19 @@ > return; > } > > + if (isFromSystemMacro(S, CC)) > + return; > + > DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_float_precision); > } > return; > } > > - // If the target is integral, always warn. > + // If the target is integral, always warn. > if ((TargetBT && TargetBT->isInteger())) { > + if (isFromSystemMacro(S, CC)) > + return; > + > Expr *InnerE = E->IgnoreParenImpCasts(); > if (FloatingLiteral *LiteralExpr = dyn_cast<FloatingLiteral>(InnerE)) { > DiagnoseImpCast(S, LiteralExpr, T, CC, > @@ -2852,6 +2871,9 @@ > // TODO: this should happen for bitfield stores, too. > llvm::APSInt Value(32); > if (E->isIntegerConstantExpr(Value, S.Context)) { > + if (isFromSystemMacro(S, CC)) > + return; > + > std::string PrettySourceValue = Value.toString(10); > std::string PrettyTargetValue = PrettyPrintInRange(Value, TargetRange); > > @@ -2863,6 +2885,10 @@ > > // People want to build with -Wshorten-64-to-32 and not -Wconversion > // and by god we'll let them. > + > + if (isFromSystemMacro(S, CC)) > + return; > + > if (SourceRange.Width == 64 && TargetRange.Width == 32) > return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_integer_64_32); > return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_integer_precision); > @@ -2871,6 +2897,10 @@ > if ((TargetRange.NonNegative && !SourceRange.NonNegative) || > (!TargetRange.NonNegative && SourceRange.NonNegative && > SourceRange.Width == TargetRange.Width)) { > + > + if (isFromSystemMacro(S, CC)) > + return; > + > unsigned DiagID = diag::warn_impcast_integer_sign; > > // Traditionally, gcc has warned about this under -Wsign-compare. > @@ -2893,9 +2923,13 @@ > SourceEnum->getDecl()->getTypedefForAnonDecl()) && > (TargetEnum->getDecl()->getIdentifier() || > TargetEnum->getDecl()->getTypedefForAnonDecl()) && > - SourceEnum != TargetEnum) > + SourceEnum != TargetEnum) { > + if (isFromSystemMacro(S, CC)) > + return; > + > return DiagnoseImpCast(S, E, T, CC, > diag::warn_impcast_different_enum_types); > + } > > return; > } > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
