[PATCH] D29240: IR: Consider two DISubprograms to be odr-equal if they have the same template parameters.

[David Blaikie via llvm-commits]

Duncan & Adrian's perspectives would be appreciated here.

I'm not sure I understood/was aware of the DISubprogram uniquing as such -
I mean I know the DISubprogram declarations are not 'distinct' so they get
uniqued in some way, but didn't realize it looked like this. Guess I had
some idea - not suggesting it's the wrong approach by any means, though.

On Fri, Jan 27, 2017 at 8:09 PM Peter Collingbourne via Phabricator via
llvm-commits <llvm-commits at lists.llvm.org> wrote:

> pcc created this revision.
>
> In ValueMapper we create new operands for MDNodes and
> rely on MDNode::replaceWithUniqued to create a new MDNode
> with the specified operands. However this doesn't always
> actually happen correctly for DISubprograms because when we
> uniquify the new node, we only odr-compare it with existing nodes
> (MDNodeSubsetEqualImpl<DISubprogram>::isDeclarationOfODRMember). Although
> the TemplateParameters field can refer to a distinct DICompileUnit via
> DITemplateTypeParameter::type -> DICompositeType::scope ->
> DISubprogram::unit,
> it is not currently included in the odr comparison. As a result, we can end
> up getting our original DISubprogram back, which means we will have a
> cloned
> module referring to the DICompileUnit in the original module, which causes
> a verification error.
>
> The fix I implemented was to consider TemplateParameters to be one of the
> odr-equal properties. But I'm a little uncomfortable with this. In general
> it
> seems unsound to rely on distinct MDNodes never being reachable from nodes
> which we only check odr-equality of. My only long term suggestion would be
> to separate odr-uniquing from full uniquing.
>
> Test case to come.
>
>
> https://reviews.llvm.org/D29240
>
> Files:
>   llvm/lib/IR/LLVMContextImpl.h
>
>
> Index: llvm/lib/IR/LLVMContextImpl.h
> ===================================================================
> --- llvm/lib/IR/LLVMContextImpl.h
> +++ llvm/lib/IR/LLVMContextImpl.h
> @@ -612,17 +612,19 @@
>    typedef MDNodeKeyImpl<DISubprogram> KeyTy;
>    static bool isSubsetEqual(const KeyTy &LHS, const DISubprogram *RHS) {
>      return isDeclarationOfODRMember(LHS.IsDefinition, LHS.Scope,
> -                                    LHS.LinkageName, RHS);
> +                                    LHS.LinkageName, LHS.TemplateParams,
> RHS);
>    }
>    static bool isSubsetEqual(const DISubprogram *LHS, const DISubprogram
> *RHS) {
>      return isDeclarationOfODRMember(LHS->isDefinition(),
> LHS->getRawScope(),
> -                                    LHS->getRawLinkageName(), RHS);
> +                                    LHS->getRawLinkageName(),
> +                                    LHS->getRawTemplateParams(), RHS);
>    }
>
>    /// Subprograms compare equal if they declare the same function in an
> ODR
>    /// type.
>    static bool isDeclarationOfODRMember(bool IsDefinition, const Metadata
> *Scope,
>                                         const MDString *LinkageName,
> +                                       const Metadata *TemplateParams,
>                                         const DISubprogram *RHS) {
>      // Check whether the LHS is eligible.
>      if (IsDefinition || !Scope || !LinkageName)
> @@ -634,7 +636,8 @@
>
>      // Compare to the RHS.
>      return IsDefinition == RHS->isDefinition() && Scope ==
> RHS->getRawScope() &&
> -           LinkageName == RHS->getRawLinkageName();
> +           LinkageName == RHS->getRawLinkageName() &&
> +           TemplateParams == RHS->getRawTemplateParams();
>    }
>  };
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170130/c94b8301/attachment.html>