[PATCH] D16571: Compute the DISubprogram hash key partially (NFC)

[Duncan P. N. Exon Smith via llvm-commits]

Sorry, I thought I'd responded back in January.  This approach SGTM.

I have some ideas about field choice below.  If you've profiled -flto -g
with something non-trivial, then I'm happy for you to commit (LGTM!), but
you might be able to extract even better performance.

> On 2016-Mar-05, at 23:13, Mehdi AMINI <mehdi.amini at apple.com> wrote:
> 
> joker.eph updated this revision to Diff 49892.
> joker.eph updated the summary for this revision.
> joker.eph added a comment.
> 
> Change the order of arguments as suggested by Eric.
> 
> Duncan?
> 
> 
> http://reviews.llvm.org/D16571
> 
> Files:
>  lib/IR/LLVMContextImpl.h
> 
> Index: lib/IR/LLVMContextImpl.h
> ===================================================================
> --- lib/IR/LLVMContextImpl.h
> +++ lib/IR/LLVMContextImpl.h
> @@ -365,8 +365,7 @@
>            ExtraData == RHS->getRawExtraData();
>   }
>   unsigned getHashValue() const {
> -    return hash_combine(Tag, Name, File, Line, Scope, BaseType, SizeInBits,
> -                        AlignInBits, OffsetInBits, Flags, ExtraData);
> +    return hash_combine(Name, File, Line, Scope, BaseType);

I suspect the most important distinguishers are:
- Tag
- Name
- BaseType
- Scope

This tells you the kind of the derived type (pointer, typedef, etc.),
its name (if any), what it's derived from, and where it's defined.  In
other words, I suggest dropping File/Line (which is often left blank)
and adding back Tag.

However, you should confirm that dropping File/Line doesn't hurt
performance of -flto=full.

>   }
> };
> 
> @@ -422,9 +421,7 @@
>            Identifier == RHS->getIdentifier();
>   }
>   unsigned getHashValue() const {
> -    return hash_combine(Tag, Name, File, Line, Scope, BaseType, SizeInBits,
> -                        AlignInBits, OffsetInBits, Flags, Elements, RuntimeLang,
> -                        VTableHolder, TemplateParams, Identifier);
> +    return hash_combine(Name, File, Line, Scope, SizeInBits, Elements);

I feel like you can safely drop SizeInBits and Elements.  I think it's
unlikely the other four fields will collide.

Again, you should profile -flto=full with -g before dropping this.

>   }
> };
> 
> @@ -518,10 +515,7 @@
>            Variables == RHS->getRawVariables();
>   }
>   unsigned getHashValue() const {
> -    return hash_combine(Scope, Name, LinkageName, File, Line, Type,
> -                        IsLocalToUnit, IsDefinition, ScopeLine, ContainingType,
> -                        Virtuality, VirtualIndex, Flags, IsOptimized,
> -                        TemplateParams, Declaration, Variables);
> +    return hash_combine(File, Line, Type, Scope);
>   }
> };

It feels strange to leave both Name and LinkageName out here.  I'm not
sure it's wrong though.  As long as you've profiled this.

> 
> 
> 
> <D16571.49892.patch>