r359960 - Reduce amount of work ODR hashing does.

[David Blaikie via cfe-commits]

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 at 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 at first.h:* {{'FunctionProtoTypeDecay::S1::Y' from module 'FirstModule' is not present in definition of 'FunctionProtoTypeDecay::S1' in module 'SecondModule'}}
> +// expected-note at 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 at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits