Does the ODR hashing have some sort of cycle breaking infrastructure - so that if the same type is seen more than once (eg: classes have members that have pointers back to the outer class type, etc) they don't cause indefinite cycles? Should that infrastructure have caught these cases & avoided the redundant work?
I'm curious to understand better how these things work/overlap/or don't. On Fri, May 3, 2019 at 9:20 PM Richard Trieu via cfe-commits <cfe-commits@lists.llvm.org> wrote: > > Author: rtrieu > Date: Fri May 3 21:22:33 2019 > New Revision: 359960 > > URL: http://llvm.org/viewvc/llvm-project?rev=359960&view=rev > Log: > Reduce amount of work ODR hashing does. > > When a FunctionProtoType is in the original type in a DecayedType, the decayed > type is a PointerType which points back the original FunctionProtoType. The > visitor for ODRHashing will attempt to process both Type's, doing double work. > By chaining together multiple DecayedType's and FunctionProtoType's, this > would > result in 2^N Type's visited only N DecayedType's and N FunctionProtoType's > exsit. Another bug where VisitDecayedType and VisitAdjustedType did > redundant work doubled the work at each level, giving 4^N Type's visited. > This > patch removed the double work and detects when a FunctionProtoType decays to > itself to only check the Type once. This lowers the exponential runtime to > linear runtime. Fixes https://bugs.llvm.org/show_bug.cgi?id=41625 > > Modified: > cfe/trunk/lib/AST/ODRHash.cpp > cfe/trunk/test/Modules/odr_hash.cpp > > Modified: cfe/trunk/lib/AST/ODRHash.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHash.cpp?rev=359960&r1=359959&r2=359960&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/ODRHash.cpp (original) > +++ cfe/trunk/lib/AST/ODRHash.cpp Fri May 3 21:22:33 2019 > @@ -703,14 +703,36 @@ public: > void VisitType(const Type *T) {} > > void VisitAdjustedType(const AdjustedType *T) { > - AddQualType(T->getOriginalType()); > - AddQualType(T->getAdjustedType()); > + QualType Original = T->getOriginalType(); > + QualType Adjusted = T->getAdjustedType(); > + > + // The original type and pointee type can be the same, as in the case of > + // function pointers decaying to themselves. Set a bool and only process > + // the type once, to prevent doubling the work. > + SplitQualType split = Adjusted.split(); > + if (auto Pointer = dyn_cast<PointerType>(split.Ty)) { > + if (Pointer->getPointeeType() == Original) { > + Hash.AddBoolean(true); > + ID.AddInteger(split.Quals.getAsOpaqueValue()); > + AddQualType(Original); > + VisitType(T); > + return; > + } > + } > + > + // The original type and pointee type are different, such as in the case > + // of a array decaying to an element pointer. Set a bool to false and > + // process both types. > + Hash.AddBoolean(false); > + AddQualType(Original); > + AddQualType(Adjusted); > + > VisitType(T); > } > > void VisitDecayedType(const DecayedType *T) { > - AddQualType(T->getDecayedType()); > - AddQualType(T->getPointeeType()); > + // getDecayedType and getPointeeType are derived from getAdjustedType > + // and don't need to be separately processed. > VisitAdjustedType(T); > } > > > Modified: cfe/trunk/test/Modules/odr_hash.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/odr_hash.cpp?rev=359960&r1=359959&r2=359960&view=diff > ============================================================================== > --- cfe/trunk/test/Modules/odr_hash.cpp (original) > +++ cfe/trunk/test/Modules/odr_hash.cpp Fri May 3 21:22:33 2019 > @@ -4587,6 +4587,43 @@ int num = bar(); > #endif > } > > +namespace FunctionProtoTypeDecay { > +#if defined(FIRST) > +struct S1 { > + struct X {}; > + using Y = X(X()); > +}; > +#elif defined(SECOND) > +struct S1 { > + struct X {}; > + using Y = X(X(X())); > +}; > +#else > +S1 s1; > +// expected-error@first.h:* {{'FunctionProtoTypeDecay::S1::Y' from module > 'FirstModule' is not present in definition of 'FunctionProtoTypeDecay::S1' in > module 'SecondModule'}} > +// expected-note@second.h:* {{declaration of 'Y' does not match}} > +#endif > + > +#if defined(FIRST) > +struct S2 { > + struct X {}; > + using Y = > + X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(X( > + X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(X( > + X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(X( > + X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(X( > + )))))))))))))))) > + )))))))))))))))) > + )))))))))))))))) > + )))))))))))))))); > +}; > +#elif defined(SECOND) > +#else > +S2 s2; > +#endif > + > +} > + > // Keep macros contained to one file. > #ifdef FIRST > #undef FIRST > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
