Hey Richard, This broke bootstrap, at least on Darwin.
addDiag() is getting called when EvalStatus.Diag is null, which == boom. Can you investigate? - Daniel On Wed, Mar 14, 2012 at 5:41 PM, Richard Smith <[email protected]> wrote: > Author: rsmith > Date: Wed Mar 14 19:41:48 2012 > New Revision: 152761 > > URL: http://llvm.org/viewvc/llvm-project?rev=152761&view=rev > Log: > Minor optimization to constant evaluation: don't bother computing expr source > locations for diagnostics we're not going to emit, and don't track the > subobject > designator outside C++11 (since we're not going to use it anyway). > > This seems to give about a 0.5% speedup on 403.gcc/combine.c, but the results > were sufficiently noisy that I can't reject the null hypothesis. > > Modified: > cfe/trunk/lib/AST/ExprConstant.cpp > > Modified: cfe/trunk/lib/AST/ExprConstant.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=152761&r1=152760&r2=152761&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/ExprConstant.cpp (original) > +++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Mar 14 19:41:48 2012 > @@ -480,9 +480,18 @@ > return OptionalDiagnostic(); > } > > + OptionalDiagnostic Diag(const Expr *E, diag::kind DiagId > + = diag::note_invalid_subexpr_in_const_expr, > + unsigned ExtraNotes = 0) { > + if (EvalStatus.Diag) > + return Diag(E->getExprLoc(), DiagId, ExtraNotes); > + return OptionalDiagnostic(); > + } > + > /// Diagnose that the evaluation does not produce a C++11 core constant > /// expression. > - OptionalDiagnostic CCEDiag(SourceLocation Loc, diag::kind DiagId > + template<typename LocArg> > + OptionalDiagnostic CCEDiag(LocArg Loc, diag::kind DiagId > = diag::note_invalid_subexpr_in_const_expr, > unsigned ExtraNotes = 0) { > // Don't override a previous diagnostic. > @@ -556,7 +565,7 @@ > if (Invalid) > return false; > if (isOnePastTheEnd()) { > - Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_past_end_subobject) > + Info.CCEDiag(E, diag::note_constexpr_past_end_subobject) > << CSK; > setInvalid(); > return false; > @@ -567,11 +576,11 @@ > void SubobjectDesignator::diagnosePointerArithmetic(EvalInfo &Info, > const Expr *E, uint64_t > N) { > if (MostDerivedPathLength == Entries.size() && MostDerivedArraySize) > - Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_array_index) > + Info.CCEDiag(E, diag::note_constexpr_array_index) > << static_cast<int>(N) << /*array*/ 0 > << static_cast<unsigned>(MostDerivedArraySize); > else > - Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_array_index) > + Info.CCEDiag(E, diag::note_constexpr_array_index) > << static_cast<int>(N) << /*non-array*/ 1; > setInvalid(); > } > @@ -730,7 +739,7 @@ > if (Designator.Invalid) > return false; > if (!Base) { > - Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_null_subobject) > + Info.CCEDiag(E, diag::note_constexpr_null_subobject) > << CSK; > Designator.setInvalid(); > return false; > @@ -741,27 +750,30 @@ > // Check this LValue refers to an object. If not, set the designator to be > // invalid and emit a diagnostic. > bool checkSubobject(EvalInfo &Info, const Expr *E, CheckSubobjectKind > CSK) { > + // Outside C++11, do not build a designator referring to a subobject of > + // any object: we won't use such a designator for anything. > + if (!Info.getLangOpts().CPlusPlus0x) > + Designator.setInvalid(); > return checkNullPointer(Info, E, CSK) && > Designator.checkSubobject(Info, E, CSK); > } > > void addDecl(EvalInfo &Info, const Expr *E, > const Decl *D, bool Virtual = false) { > - checkSubobject(Info, E, isa<FieldDecl>(D) ? CSK_Field : CSK_Base); > - Designator.addDeclUnchecked(D, Virtual); > + if (checkSubobject(Info, E, isa<FieldDecl>(D) ? CSK_Field : CSK_Base)) > + Designator.addDeclUnchecked(D, Virtual); > } > void addArray(EvalInfo &Info, const Expr *E, const ConstantArrayType > *CAT) { > - checkSubobject(Info, E, CSK_ArrayToPointer); > - Designator.addArrayUnchecked(CAT); > + if (checkSubobject(Info, E, CSK_ArrayToPointer)) > + Designator.addArrayUnchecked(CAT); > } > void addComplex(EvalInfo &Info, const Expr *E, QualType EltTy, bool Imag) > { > - checkSubobject(Info, E, Imag ? CSK_Imag : CSK_Real); > - Designator.addComplexUnchecked(EltTy, Imag); > + if (checkSubobject(Info, E, Imag ? CSK_Imag : CSK_Real)) > + Designator.addComplexUnchecked(EltTy, Imag); > } > void adjustIndex(EvalInfo &Info, const Expr *E, uint64_t N) { > - if (!checkNullPointer(Info, E, CSK_ArrayIndex)) > - return; > - Designator.adjustIndex(Info, E, N); > + if (checkNullPointer(Info, E, CSK_ArrayIndex)) > + Designator.adjustIndex(Info, E, N); > } > }; > > @@ -1020,10 +1032,10 @@ > > // Prvalue constant expressions must be of literal types. > if (Info.getLangOpts().CPlusPlus0x) > - Info.Diag(E->getExprLoc(), diag::note_constexpr_nonliteral) > + Info.Diag(E, diag::note_constexpr_nonliteral) > << E->getType(); > else > - Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr); > + Info.Diag(E, diag::note_invalid_subexpr_in_const_expr); > return false; > } > > @@ -1155,7 +1167,7 @@ > template<typename T> > static bool HandleOverflow(EvalInfo &Info, const Expr *E, > const T &SrcValue, QualType DestType) { > - Info.Diag(E->getExprLoc(), diag::note_constexpr_overflow) > + Info.Diag(E, diag::note_constexpr_overflow) > << SrcValue << DestType; > return false; > } > @@ -1240,7 +1252,7 @@ > } else { > // Don't try to handle vectors of anything other than int or float > // (not sure if it's possible to hit this case). > - Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr); > + Info.Diag(E, diag::note_invalid_subexpr_in_const_expr); > return false; > } > unsigned BaseEltSize = EltAsInt.getBitWidth(); > @@ -1253,7 +1265,7 @@ > } > // Give up if the input isn't an int, float, or vector. For example, we > // reject "(v4i16)(intptr_t)&a". > - Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr); > + Info.Diag(E, diag::note_invalid_subexpr_in_const_expr); > return false; > } > > @@ -1414,7 +1426,7 @@ > if (Info.CheckingPotentialConstantExpression) > return false; > if (!Frame || !Frame->Arguments) { > - Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr); > + Info.Diag(E, diag::note_invalid_subexpr_in_const_expr); > return false; > } > Result = Frame->Arguments[PVD->getFunctionScopeIndex()]; > @@ -1427,7 +1439,7 @@ > // If we're checking a potential constant expression, the variable could > be > // initialized later. > if (!Info.CheckingPotentialConstantExpression) > - Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr); > + Info.Diag(E, diag::note_invalid_subexpr_in_const_expr); > return false; > } > > @@ -1441,7 +1453,7 @@ > // Never evaluate the initializer of a weak variable. We can't be sure that > // this is the definition which will be used. > if (VD->isWeak()) { > - Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr); > + Info.Diag(E, diag::note_invalid_subexpr_in_const_expr); > return false; > } > > @@ -1449,13 +1461,13 @@ > // this in the cases where it matters for conformance. > llvm::SmallVector<PartialDiagnosticAt, 8> Notes; > if (!VD->evaluateValue(Notes)) { > - Info.Diag(E->getExprLoc(), diag::note_constexpr_var_init_non_constant, > + Info.Diag(E, diag::note_constexpr_var_init_non_constant, > Notes.size() + 1) << VD; > Info.Note(VD->getLocation(), diag::note_declared_at); > Info.addNotes(Notes); > return false; > } else if (!VD->checkInitIsICE()) { > - Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_var_init_non_constant, > + Info.CCEDiag(E, diag::note_constexpr_var_init_non_constant, > Notes.size() + 1) << VD; > Info.Note(VD->getLocation(), diag::note_declared_at); > Info.addNotes(Notes); > @@ -1507,7 +1519,7 @@ > // A diagnostic will have already been produced. > return false; > if (Sub.isOnePastTheEnd()) { > - Info.Diag(E->getExprLoc(), Info.getLangOpts().CPlusPlus0x ? > + Info.Diag(E, Info.getLangOpts().CPlusPlus0x ? > (unsigned)diag::note_constexpr_read_past_end : > (unsigned)diag::note_invalid_subexpr_in_const_expr); > return false; > @@ -1529,7 +1541,7 @@ > if (CAT->getSize().ule(Index)) { > // Note, it should not be possible to form a pointer with a valid > // designator which points more than one past the end of the array. > - Info.Diag(E->getExprLoc(), Info.getLangOpts().CPlusPlus0x ? > + Info.Diag(E, Info.getLangOpts().CPlusPlus0x ? > (unsigned)diag::note_constexpr_read_past_end : > (unsigned)diag::note_invalid_subexpr_in_const_expr); > return false; > @@ -1551,7 +1563,7 @@ > // Next subobject is a complex number. > uint64_t Index = Sub.Entries[I].ArrayIndex; > if (Index > 1) { > - Info.Diag(E->getExprLoc(), Info.getLangOpts().CPlusPlus0x ? > + Info.Diag(E, Info.getLangOpts().CPlusPlus0x ? > (unsigned)diag::note_constexpr_read_past_end : > (unsigned)diag::note_invalid_subexpr_in_const_expr); > return false; > @@ -1568,7 +1580,7 @@ > return true; > } else if (const FieldDecl *Field = getAsField(Sub.Entries[I])) { > if (Field->isMutable()) { > - Info.Diag(E->getExprLoc(), diag::note_constexpr_ltor_mutable, 1) > + Info.Diag(E, diag::note_constexpr_ltor_mutable, 1) > << Field; > Info.Note(Field->getLocation(), diag::note_declared_at); > return false; > @@ -1580,8 +1592,7 @@ > const FieldDecl *UnionField = O->getUnionField(); > if (!UnionField || > UnionField->getCanonicalDecl() != Field->getCanonicalDecl()) { > - Info.Diag(E->getExprLoc(), > - diag::note_constexpr_read_inactive_union_member) > + Info.Diag(E, diag::note_constexpr_read_inactive_union_member) > << Field << !UnionField << UnionField; > return false; > } > @@ -1593,11 +1604,11 @@ > if (ObjType.isVolatileQualified()) { > if (Info.getLangOpts().CPlusPlus) { > // FIXME: Include a description of the path to the volatile > subobject. > - Info.Diag(E->getExprLoc(), diag::note_constexpr_ltor_volatile_obj, > 1) > + Info.Diag(E, diag::note_constexpr_ltor_volatile_obj, 1) > << 2 << Field; > Info.Note(Field->getLocation(), diag::note_declared_at); > } else { > - Info.Diag(E->getExprLoc(), > diag::note_invalid_subexpr_in_const_expr); > + Info.Diag(E, diag::note_invalid_subexpr_in_const_expr); > } > return false; > } > @@ -1611,7 +1622,7 @@ > > if (O->isUninit()) { > if (!Info.CheckingPotentialConstantExpression) > - Info.Diag(E->getExprLoc(), diag::note_constexpr_read_uninit); > + Info.Diag(E, diag::note_constexpr_read_uninit); > return false; > } > } > @@ -1702,11 +1713,10 @@ > return false; > > const Expr *Base = LVal.Base.dyn_cast<const Expr*>(); > - SourceLocation Loc = Conv->getExprLoc(); > > if (!LVal.Base) { > // FIXME: Indirection through a null pointer deserves a specific > diagnostic. > - Info.Diag(Loc, diag::note_invalid_subexpr_in_const_expr); > + Info.Diag(Conv, diag::note_invalid_subexpr_in_const_expr); > return false; > } > > @@ -1714,7 +1724,7 @@ > if (LVal.CallIndex) { > Frame = Info.getCallFrame(LVal.CallIndex); > if (!Frame) { > - Info.Diag(Loc, diag::note_constexpr_lifetime_ended, 1) << !Base; > + Info.Diag(Conv, diag::note_constexpr_lifetime_ended, 1) << !Base; > NoteLValueLocation(Info, LVal.Base); > return false; > } > @@ -1726,9 +1736,9 @@ > // semantics. > if (Type.isVolatileQualified()) { > if (Info.getLangOpts().CPlusPlus) > - Info.Diag(Loc, diag::note_constexpr_ltor_volatile_type) << Type; > + Info.Diag(Conv, diag::note_constexpr_ltor_volatile_type) << Type; > else > - Info.Diag(Loc); > + Info.Diag(Conv); > return false; > } > > @@ -1742,7 +1752,7 @@ > if (const VarDecl *VDef = VD->getDefinition(Info.Ctx)) > VD = VDef; > if (!VD || VD->isInvalidDecl()) { > - Info.Diag(Loc); > + Info.Diag(Conv); > return false; > } > > @@ -1751,10 +1761,10 @@ > QualType VT = VD->getType(); > if (VT.isVolatileQualified()) { > if (Info.getLangOpts().CPlusPlus) { > - Info.Diag(Loc, diag::note_constexpr_ltor_volatile_obj, 1) << 1 << VD; > + Info.Diag(Conv, diag::note_constexpr_ltor_volatile_obj, 1) << 1 << > VD; > Info.Note(VD->getLocation(), diag::note_declared_at); > } else { > - Info.Diag(Loc); > + Info.Diag(Conv); > } > return false; > } > @@ -1765,10 +1775,10 @@ > } else if (VT->isIntegralOrEnumerationType()) { > if (!VT.isConstQualified()) { > if (Info.getLangOpts().CPlusPlus) { > - Info.Diag(Loc, diag::note_constexpr_ltor_non_const_int, 1) << VD; > + Info.Diag(Conv, diag::note_constexpr_ltor_non_const_int, 1) << > VD; > Info.Note(VD->getLocation(), diag::note_declared_at); > } else { > - Info.Diag(Loc); > + Info.Diag(Conv); > } > return false; > } > @@ -1777,18 +1787,18 @@ > // static const data members of such types (supported as an extension) > // more useful. > if (Info.getLangOpts().CPlusPlus0x) { > - Info.CCEDiag(Loc, diag::note_constexpr_ltor_non_constexpr, 1) << > VD; > + Info.CCEDiag(Conv, diag::note_constexpr_ltor_non_constexpr, 1) << > VD; > Info.Note(VD->getLocation(), diag::note_declared_at); > } else { > - Info.CCEDiag(Loc); > + Info.CCEDiag(Conv); > } > } else { > // FIXME: Allow folding of values of any literal type in all > languages. > if (Info.getLangOpts().CPlusPlus0x) { > - Info.Diag(Loc, diag::note_constexpr_ltor_non_constexpr, 1) << VD; > + Info.Diag(Conv, diag::note_constexpr_ltor_non_constexpr, 1) << VD; > Info.Note(VD->getLocation(), diag::note_declared_at); > } else { > - Info.Diag(Loc); > + Info.Diag(Conv); > } > return false; > } > @@ -1812,7 +1822,7 @@ > if (unsigned CallIndex = RVal.getLValueCallIndex()) { > Frame = Info.getCallFrame(CallIndex); > if (!Frame) { > - Info.Diag(Loc, diag::note_constexpr_lifetime_ended, 1) << !Base; > + Info.Diag(Conv, diag::note_constexpr_lifetime_ended, 1) << !Base; > NoteLValueLocation(Info, RVal.getLValueBase()); > return false; > } > @@ -1824,10 +1834,10 @@ > // Volatile temporary objects cannot be read in constant expressions. > if (Base->getType().isVolatileQualified()) { > if (Info.getLangOpts().CPlusPlus) { > - Info.Diag(Loc, diag::note_constexpr_ltor_volatile_obj, 1) << 0; > + Info.Diag(Conv, diag::note_constexpr_ltor_volatile_obj, 1) << 0; > Info.Note(Base->getExprLoc(), diag::note_constexpr_temporary_here); > } else { > - Info.Diag(Loc); > + Info.Diag(Conv); > } > return false; > } > @@ -1850,7 +1860,7 @@ > // FIXME: Support PredefinedExpr, ObjCEncodeExpr, MakeStringConstant > RVal = APValue(Base, CharUnits::Zero(), APValue::NoLValuePath(), 0); > } else { > - Info.Diag(Conv->getExprLoc(), diag::note_invalid_subexpr_in_const_expr); > + Info.Diag(Conv, diag::note_invalid_subexpr_in_const_expr); > return false; > } > > @@ -1976,7 +1986,7 @@ > > // Check this cast lands within the final derived-to-base subobject path. > if (D.MostDerivedPathLength + E->path_size() > D.Entries.size()) { > - Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_invalid_downcast) > + Info.CCEDiag(E, diag::note_constexpr_invalid_downcast) > << D.MostDerivedType << TargetQT; > return false; > } > @@ -1991,7 +2001,7 @@ > else > FinalType = getAsBaseClass(D.Entries[NewEntriesSize - 1]); > if (FinalType->getCanonicalDecl() != TargetType->getCanonicalDecl()) { > - Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_invalid_downcast) > + Info.CCEDiag(E, diag::note_constexpr_invalid_downcast) > << D.MostDerivedType << TargetQT; > return false; > } > @@ -2420,13 +2430,13 @@ > typedef ExprEvaluatorBase ExprEvaluatorBaseTy; > > OptionalDiagnostic CCEDiag(const Expr *E, diag::kind D) { > - return Info.CCEDiag(E->getExprLoc(), D); > + return Info.CCEDiag(E, D); > } > > /// Report an evaluation error. This should only be called when an error is > /// first discovered. When propagating an error, just return false. > bool Error(const Expr *E, diag::kind D) { > - Info.Diag(E->getExprLoc(), D); > + Info.Diag(E, D); > return false; > } > bool Error(const Expr *E) { > @@ -2957,7 +2967,7 @@ > return Success(E); > CXXRecordDecl *RD = E->getExprOperand()->getType()->getAsCXXRecordDecl(); > if (RD && RD->isPolymorphic()) { > - Info.Diag(E->getExprLoc(), diag::note_constexpr_typeid_polymorphic) > + Info.Diag(E, diag::note_constexpr_typeid_polymorphic) > << E->getExprOperand()->getType() > << E->getExprOperand()->getSourceRange(); > return false; > @@ -3405,7 +3415,7 @@ > } > > if (isa<CXXRecordDecl>(RD) && cast<CXXRecordDecl>(RD)->getNumVBases()) { > - Info.Diag(E->getExprLoc(), diag::note_constexpr_virtual_base) << RD; > + Info.Diag(E, diag::note_constexpr_virtual_base) << RD; > return false; > } > > @@ -4115,7 +4125,7 @@ > if (!Val.isInt()) { > // FIXME: It would be better to produce the diagnostic for casting > // a pointer to an integer. > - Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr); > + Info.Diag(E, diag::note_invalid_subexpr_in_const_expr); > return false; > } > Result = Val.getInt(); > @@ -4338,10 +4348,10 @@ > case Builtin::BIstrlen: > // A call to strlen is not a constant expression. > if (Info.getLangOpts().CPlusPlus0x) > - Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_invalid_function) > + Info.CCEDiag(E, diag::note_constexpr_invalid_function) > << /*isConstexpr*/0 << /*isConstructor*/0 << "'strlen'"; > else > - Info.CCEDiag(E->getExprLoc(), > diag::note_invalid_subexpr_in_const_expr); > + Info.CCEDiag(E, diag::note_invalid_subexpr_in_const_expr); > // Fall through. > case Builtin::BI__builtin_strlen: > // As an extension, we support strlen() and __builtin_strlen() as constant > @@ -5981,17 +5991,17 @@ > Result = Info.CurrentCall->Temporaries[E]; > } else if (E->getType()->isVoidType()) { > if (Info.getLangOpts().CPlusPlus0x) > - Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_nonliteral) > + Info.CCEDiag(E, diag::note_constexpr_nonliteral) > << E->getType(); > else > - Info.CCEDiag(E->getExprLoc(), > diag::note_invalid_subexpr_in_const_expr); > + Info.CCEDiag(E, diag::note_invalid_subexpr_in_const_expr); > if (!EvaluateVoid(E, Info)) > return false; > } else if (Info.getLangOpts().CPlusPlus0x) { > - Info.Diag(E->getExprLoc(), diag::note_constexpr_nonliteral) << > E->getType(); > + Info.Diag(E, diag::note_constexpr_nonliteral) << E->getType(); > return false; > } else { > - Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr); > + Info.Diag(E, diag::note_invalid_subexpr_in_const_expr); > return false; > } > > > > _______________________________________________ > 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
