[PATCH] D16560: Hash DISubprogram Metadata using pointer for MDString argument instead of value (NFC)

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 25 15:17:39 PST 2016


Nice cleanup, assuming the tests pass.  LGTM, as long you follow up
by doing the rest of the MDNodeKeyImpls so they aren't inconsistent.
In fact, I don't really see a problem with doing them in bulk, but
split it up how you like.

> On 2016-Jan-25, at 15:06, Mehdi AMINI <mehdi.amini at apple.com> wrote:
> 
> joker.eph created this revision.
> joker.eph added a reviewer: dexonsmith.
> joker.eph added a subscriber: llvm-commits.
> 
> MDString are uniqued in the Context on creation, hashing the
> pointer is less expensive than hashing the String itself.
> 
> http://reviews.llvm.org/D16560
> 
> Files:
>  lib/IR/DebugInfoMetadata.cpp
>  lib/IR/LLVMContextImpl.h
> 
> Index: lib/IR/LLVMContextImpl.h
> ===================================================================
> --- lib/IR/LLVMContextImpl.h
> +++ lib/IR/LLVMContextImpl.h
> @@ -460,8 +460,8 @@
> 
> template <> struct MDNodeKeyImpl<DISubprogram> {
>   Metadata *Scope;
> -  StringRef Name;
> -  StringRef LinkageName;
> +  MDString *Name;
> +  MDString *LinkageName;
>   Metadata *File;
>   unsigned Line;
>   Metadata *Type;
> @@ -477,7 +477,7 @@
>   Metadata *Declaration;
>   Metadata *Variables;
> 
> -  MDNodeKeyImpl(Metadata *Scope, StringRef Name, StringRef LinkageName,
> +  MDNodeKeyImpl(Metadata *Scope, MDString *Name, MDString *LinkageName,
>                 Metadata *File, unsigned Line, Metadata *Type,
>                 bool IsLocalToUnit, bool IsDefinition, unsigned ScopeLine,
>                 Metadata *ContainingType, unsigned Virtuality,
> @@ -492,8 +492,8 @@
>         TemplateParams(TemplateParams), Declaration(Declaration),
>         Variables(Variables) {}
>   MDNodeKeyImpl(const DISubprogram *N)
> -      : Scope(N->getRawScope()), Name(N->getName()),
> -        LinkageName(N->getLinkageName()), File(N->getRawFile()),
> +      : Scope(N->getRawScope()), Name(N->getRawName()),
> +        LinkageName(N->getRawLinkageName()), File(N->getRawFile()),
>         Line(N->getLine()), Type(N->getRawType()),
>         IsLocalToUnit(N->isLocalToUnit()), IsDefinition(N->isDefinition()),
>         ScopeLine(N->getScopeLine()), ContainingType(N->getRawContainingType()),
> @@ -503,8 +503,8 @@
>         Declaration(N->getRawDeclaration()), Variables(N->getRawVariables()) {}
> 
>   bool isKeyOf(const DISubprogram *RHS) const {
> -    return Scope == RHS->getRawScope() && Name == RHS->getName() &&
> -           LinkageName == RHS->getLinkageName() && File == RHS->getRawFile() &&
> +    return Scope == RHS->getRawScope() && Name == RHS->getRawName() &&
> +           LinkageName == RHS->getRawLinkageName() && File == RHS->getRawFile() &&
>            Line == RHS->getLine() && Type == RHS->getRawType() &&
>            IsLocalToUnit == RHS->isLocalToUnit() &&
>            IsDefinition == RHS->isDefinition() &&
> Index: lib/IR/DebugInfoMetadata.cpp
> ===================================================================
> --- lib/IR/DebugInfoMetadata.cpp
> +++ lib/IR/DebugInfoMetadata.cpp
> @@ -348,7 +348,7 @@
>   assert(isCanonical(Name) && "Expected canonical MDString");
>   assert(isCanonical(LinkageName) && "Expected canonical MDString");
>   DEFINE_GETIMPL_LOOKUP(DISubprogram,
> -                        (Scope, getString(Name), getString(LinkageName), File,
> +                        (Scope, Name, LinkageName, File,
>                          Line, Type, IsLocalToUnit, IsDefinition, ScopeLine,
>                          ContainingType, Virtuality, VirtualIndex, Flags,
>                          IsOptimized, TemplateParams, Declaration, Variables));
> 
> 
> <D16560.45921.patch>



More information about the llvm-commits mailing list