<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 12, 2015 at 1:09 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
> On 2015-Jan-12, at 12:59, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Mon, Jan 12, 2015 at 11:16 AM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
><br>
> > On 2015-Jan-12, at 11:07, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> ><br>
> ><br>
> ><br>
> > On Wed, Jan 7, 2015 at 1:35 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> > Author: dexonsmith<br>
> > Date: Wed Jan 7 15:35:38 2015<br>
> > New Revision: 225401<br>
> ><br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=225401&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=225401&view=rev</a><br>
> > Log:<br>
> > IR: Add MDNode::isDistinct()<br>
> ><br>
> > Add API to indicate whether an `MDNode` is distinct. A distinct node is<br>
> > not stored in the MDNode uniquing tables, and will never be returned by<br>
> > `MDNode::get()`.<br>
> ><br>
> > Although distinct nodes are only currently created by uniquing<br>
> > collisions (when operands change),<br>
> ><br>
> > I don't quite understand how changing an operand relates to uniquing collisions - once the operand has changed would we not want the uniquing to happen again? (eg: if changing an operand made this node equal to some other node instead?)<br>
> ><br>
><br>
> It's impossible without RAUW.<br>
><br>
> Even when doing that, it seems strange that it would prevent uniquing - but again, I guess I'm missing something.<br>
><br>
> I would sort of expect:<br>
><br>
> {1, 2}<br>
> 1 = {a}<br>
> 2 = {b}<br>
><br>
> RAUW a -> b<br>
><br>
> {2, 2}<br>
> 2 = {b}<br>
><br>
> it sort of seems strange<br>
<br>
</div></div>It is strange :/.<br>
<span class=""><br>
> to me that the path (directly constructing, versus constructing then RAUWing) to create the value produces a different graph/node/etc?<br>
><br>
> - David<br>
<br>
</span>It's unavoidable though. Concrete example:<br>
<br>
@g1 = global i32 0<br>
@g2 = global i32 0<br>
<br>
!0 = !{i32* @g1}<br>
!1 = !{i32* @g2}<br>
!2 = !{!0, !1}<br>
<br>
`@g1->replaceAllUsesWith(@g2)` calls `!0->handleChangedOperand(0, @g2)`.<br>
<br>
In there, we call `MDNode::getIfExists(Context, @g2)` and get back `!1`.<br>
This is a uniquing collision.<br>
<br>
`!0` isn't tracking its users, </blockquote><div><br></div><div>Would this have been true back before the metadata/value split - is this a change in behavior since then? (I assume not, because I imagine that would break things)<br><br>Values no their uses, right? (but not their metadata uses, even before the metadata/value split?)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">so it doesn't know about `!2`. We'd really<br>
like to call `!0->replaceAllUsesWith(!1)`, but we can't.<br>
<br>
Instead, we're left with this:<br>
<br>
@g1 = global i32 0<br>
@g2 = global i32 0<br>
<br>
!0 = distinct !{i32* @g2}<br>
!1 = !{i32* @g2}<br>
!2 = !{!0, !1}<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
><br>
> > PR22111 will allow these nodes to be<br>
> > explicitly created.<br>
> ><br>
> > Modified:<br>
> > llvm/trunk/include/llvm/IR/Metadata.h<br>
> > llvm/trunk/unittests/IR/MetadataTest.cpp<br>
> ><br>
> > Modified: llvm/trunk/include/llvm/IR/Metadata.h<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Metadata.h?rev=225401&r1=225400&r2=225401&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Metadata.h?rev=225401&r1=225400&r2=225401&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/include/llvm/IR/Metadata.h (original)<br>
> > +++ llvm/trunk/include/llvm/IR/Metadata.h Wed Jan 7 15:35:38 2015<br>
> > @@ -642,6 +642,12 @@ public:<br>
> > /// \brief Check if node is fully resolved.<br>
> > bool isResolved() const;<br>
> ><br>
> > + /// \brief Check if node is distinct.<br>
> > + ///<br>
> > + /// Distinct nodes are not uniqued, and will not be returned by \a<br>
> > + /// MDNode::get().<br>
> > + bool isDistinct() const { return IsDistinctInContext; }<br>
> > +<br>
> > protected:<br>
> > /// \brief Set an operand.<br>
> > ///<br>
> ><br>
> > Modified: llvm/trunk/unittests/IR/MetadataTest.cpp<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/MetadataTest.cpp?rev=225401&r1=225400&r2=225401&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/MetadataTest.cpp?rev=225401&r1=225400&r2=225401&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/unittests/IR/MetadataTest.cpp (original)<br>
> > +++ llvm/trunk/unittests/IR/MetadataTest.cpp Wed Jan 7 15:35:38 2015<br>
> > @@ -222,6 +222,33 @@ TEST_F(MDNodeTest, NullOperand) {<br>
> > EXPECT_EQ(N, NullOp);<br>
> > }<br>
> ><br>
> > +TEST_F(MDNodeTest, DistinctOnUniquingCollision) {<br>
> > + // !{}<br>
> > + MDNode *Empty = MDNode::get(Context, None);<br>
> > + ASSERT_TRUE(Empty->isResolved());<br>
> > + EXPECT_FALSE(Empty->isDistinct());<br>
> > +<br>
> > + // !{!{}}<br>
> > + Metadata *Wrapped1Ops[] = {Empty};<br>
> > + MDNode *Wrapped1 = MDNode::get(Context, Wrapped1Ops);<br>
> > + ASSERT_EQ(Empty, Wrapped1->getOperand(0));<br>
> > + ASSERT_TRUE(Wrapped1->isResolved());<br>
> > + EXPECT_FALSE(Wrapped1->isDistinct());<br>
> > +<br>
> > + // !{!{!{}}}<br>
> > + Metadata *Wrapped2Ops[] = {Wrapped1};<br>
> > + MDNode *Wrapped2 = MDNode::get(Context, Wrapped2Ops);<br>
> > + ASSERT_EQ(Wrapped1, Wrapped2->getOperand(0));<br>
> > + ASSERT_TRUE(Wrapped2->isResolved());<br>
> > + EXPECT_FALSE(Wrapped2->isDistinct());<br>
> > +<br>
> > + // !{!{!{}}} => !{!{}}<br>
> > + Wrapped2->replaceOperandWith(0, Empty);<br>
> > + ASSERT_EQ(Empty, Wrapped2->getOperand(0));<br>
> > + EXPECT_TRUE(Wrapped2->isDistinct());<br>
> > + EXPECT_FALSE(Wrapped1->isDistinct());<br>
> > +}<br>
> > +<br>
> > typedef MetadataTest MetadataAsValueTest;<br>
> ><br>
> > TEST_F(MetadataAsValueTest, MDNode) {<br>
> ><br>
> ><br>
> > _______________________________________________<br>
> > llvm-commits mailing list<br>
> > <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
> ><br>
><br>
><br>
<br>
</div></div></blockquote></div><br></div></div>