<div dir="ltr">Duncan & Adrian's perspectives would be appreciated here.<br><br>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.<br><br><div class="gmail_quote"><div dir="ltr">On Fri, Jan 27, 2017 at 8:09 PM Peter Collingbourne via Phabricator via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">pcc created this revision.<br class="gmail_msg">
<br class="gmail_msg">
In ValueMapper we create new operands for MDNodes and<br class="gmail_msg">
rely on MDNode::replaceWithUniqued to create a new MDNode<br class="gmail_msg">
with the specified operands. However this doesn't always<br class="gmail_msg">
actually happen correctly for DISubprograms because when we<br class="gmail_msg">
uniquify the new node, we only odr-compare it with existing nodes<br class="gmail_msg">
(MDNodeSubsetEqualImpl<DISubprogram>::isDeclarationOfODRMember). Although<br class="gmail_msg">
the TemplateParameters field can refer to a distinct DICompileUnit via<br class="gmail_msg">
DITemplateTypeParameter::type -> DICompositeType::scope -> DISubprogram::unit,<br class="gmail_msg">
it is not currently included in the odr comparison. As a result, we can end<br class="gmail_msg">
up getting our original DISubprogram back, which means we will have a cloned<br class="gmail_msg">
module referring to the DICompileUnit in the original module, which causes<br class="gmail_msg">
a verification error.<br class="gmail_msg">
<br class="gmail_msg">
The fix I implemented was to consider TemplateParameters to be one of the<br class="gmail_msg">
odr-equal properties. But I'm a little uncomfortable with this. In general it<br class="gmail_msg">
seems unsound to rely on distinct MDNodes never being reachable from nodes<br class="gmail_msg">
which we only check odr-equality of. My only long term suggestion would be<br class="gmail_msg">
to separate odr-uniquing from full uniquing.<br class="gmail_msg">
<br class="gmail_msg">
Test case to come.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://reviews.llvm.org/D29240" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D29240</a><br class="gmail_msg">
<br class="gmail_msg">
Files:<br class="gmail_msg">
  llvm/lib/IR/LLVMContextImpl.h<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
Index: llvm/lib/IR/LLVMContextImpl.h<br class="gmail_msg">
===================================================================<br class="gmail_msg">
--- llvm/lib/IR/LLVMContextImpl.h<br class="gmail_msg">
+++ llvm/lib/IR/LLVMContextImpl.h<br class="gmail_msg">
@@ -612,17 +612,19 @@<br class="gmail_msg">
   typedef MDNodeKeyImpl<DISubprogram> KeyTy;<br class="gmail_msg">
   static bool isSubsetEqual(const KeyTy &LHS, const DISubprogram *RHS) {<br class="gmail_msg">
     return isDeclarationOfODRMember(LHS.IsDefinition, LHS.Scope,<br class="gmail_msg">
-                                    LHS.LinkageName, RHS);<br class="gmail_msg">
+                                    LHS.LinkageName, LHS.TemplateParams, RHS);<br class="gmail_msg">
   }<br class="gmail_msg">
   static bool isSubsetEqual(const DISubprogram *LHS, const DISubprogram *RHS) {<br class="gmail_msg">
     return isDeclarationOfODRMember(LHS->isDefinition(), LHS->getRawScope(),<br class="gmail_msg">
-                                    LHS->getRawLinkageName(), RHS);<br class="gmail_msg">
+                                    LHS->getRawLinkageName(),<br class="gmail_msg">
+                                    LHS->getRawTemplateParams(), RHS);<br class="gmail_msg">
   }<br class="gmail_msg">
<br class="gmail_msg">
   /// Subprograms compare equal if they declare the same function in an ODR<br class="gmail_msg">
   /// type.<br class="gmail_msg">
   static bool isDeclarationOfODRMember(bool IsDefinition, const Metadata *Scope,<br class="gmail_msg">
                                        const MDString *LinkageName,<br class="gmail_msg">
+                                       const Metadata *TemplateParams,<br class="gmail_msg">
                                        const DISubprogram *RHS) {<br class="gmail_msg">
     // Check whether the LHS is eligible.<br class="gmail_msg">
     if (IsDefinition || !Scope || !LinkageName)<br class="gmail_msg">
@@ -634,7 +636,8 @@<br class="gmail_msg">
<br class="gmail_msg">
     // Compare to the RHS.<br class="gmail_msg">
     return IsDefinition == RHS->isDefinition() && Scope == RHS->getRawScope() &&<br class="gmail_msg">
-           LinkageName == RHS->getRawLinkageName();<br class="gmail_msg">
+           LinkageName == RHS->getRawLinkageName() &&<br class="gmail_msg">
+           TemplateParams == RHS->getRawTemplateParams();<br class="gmail_msg">
   }<br class="gmail_msg">
 };<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
_______________________________________________<br class="gmail_msg">
llvm-commits mailing list<br class="gmail_msg">
<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a><br class="gmail_msg">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="gmail_msg">
</blockquote></div></div>