[llvm] r225401 - IR: Add MDNode::isDistinct()

[David Blaikie]

On Mon, Jan 12, 2015 at 1:09 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> > On 2015-Jan-12, at 12:59, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> >
> > On Mon, Jan 12, 2015 at 11:16 AM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> >
> > > On 2015-Jan-12, at 11:07, David Blaikie <dblaikie at gmail.com> wrote:
> > >
> > >
> > >
> > > On Wed, Jan 7, 2015 at 1:35 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> > > Author: dexonsmith
> > > Date: Wed Jan  7 15:35:38 2015
> > > New Revision: 225401
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=225401&view=rev
> > > Log:
> > > IR: Add MDNode::isDistinct()
> > >
> > > Add API to indicate whether an `MDNode` is distinct.  A distinct node
> is
> > > not stored in the MDNode uniquing tables, and will never be returned by
> > > `MDNode::get()`.
> > >
> > > Although distinct nodes are only currently created by uniquing
> > > collisions (when operands change),
> > >
> > > 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?)
> > >
> >
> > It's impossible without RAUW.
> >
> > Even when doing that, it seems strange that it would prevent uniquing -
> but again, I guess I'm missing something.
> >
> > I would sort of expect:
> >
> > {1, 2}
> > 1 = {a}
> > 2 = {b}
> >
> > RAUW a -> b
> >
> > {2, 2}
> > 2 = {b}
> >
> > it sort of seems strange
>
> It is strange :/.
>
> >  to me that the path (directly constructing, versus constructing then
> RAUWing) to create the value produces a different graph/node/etc?
> >
> > - David
>
> It's unavoidable though.  Concrete example:
>
>     @g1 = global i32 0
>     @g2 = global i32 0
>
>     !0 = !{i32* @g1}
>     !1 = !{i32* @g2}
>     !2 = !{!0, !1}
>
> `@g1->replaceAllUsesWith(@g2)` calls `!0->handleChangedOperand(0, @g2)`.
>
> In there, we call `MDNode::getIfExists(Context, @g2)` and get back `!1`.
> This is a uniquing collision.
>
> `!0` isn't tracking its users,


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)

Values no their uses, right? (but not their metadata uses, even before the
metadata/value split?)


> so it doesn't know about `!2`.  We'd really
> like to call `!0->replaceAllUsesWith(!1)`, but we can't.
>
> Instead, we're left with this:
>
>     @g1 = global i32 0
>     @g2 = global i32 0
>
>     !0 = distinct !{i32* @g2}
>     !1 = !{i32* @g2}
>     !2 = !{!0, !1}
>
> >
> >
> > > PR22111 will allow these nodes to be
> > > explicitly created.
> > >
> > > Modified:
> > >     llvm/trunk/include/llvm/IR/Metadata.h
> > >     llvm/trunk/unittests/IR/MetadataTest.cpp
> > >
> > > Modified: llvm/trunk/include/llvm/IR/Metadata.h
> > > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Metadata.h?rev=225401&r1=225400&r2=225401&view=diff
> > >
> ==============================================================================
> > > --- llvm/trunk/include/llvm/IR/Metadata.h (original)
> > > +++ llvm/trunk/include/llvm/IR/Metadata.h Wed Jan  7 15:35:38 2015
> > > @@ -642,6 +642,12 @@ public:
> > >    /// \brief Check if node is fully resolved.
> > >    bool isResolved() const;
> > >
> > > +  /// \brief Check if node is distinct.
> > > +  ///
> > > +  /// Distinct nodes are not uniqued, and will not be returned by \a
> > > +  /// MDNode::get().
> > > +  bool isDistinct() const { return IsDistinctInContext; }
> > > +
> > >  protected:
> > >    /// \brief Set an operand.
> > >    ///
> > >
> > > Modified: llvm/trunk/unittests/IR/MetadataTest.cpp
> > > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/MetadataTest.cpp?rev=225401&r1=225400&r2=225401&view=diff
> > >
> ==============================================================================
> > > --- llvm/trunk/unittests/IR/MetadataTest.cpp (original)
> > > +++ llvm/trunk/unittests/IR/MetadataTest.cpp Wed Jan  7 15:35:38 2015
> > > @@ -222,6 +222,33 @@ TEST_F(MDNodeTest, NullOperand) {
> > >    EXPECT_EQ(N, NullOp);
> > >  }
> > >
> > > +TEST_F(MDNodeTest, DistinctOnUniquingCollision) {
> > > +  // !{}
> > > +  MDNode *Empty = MDNode::get(Context, None);
> > > +  ASSERT_TRUE(Empty->isResolved());
> > > +  EXPECT_FALSE(Empty->isDistinct());
> > > +
> > > +  // !{!{}}
> > > +  Metadata *Wrapped1Ops[] = {Empty};
> > > +  MDNode *Wrapped1 = MDNode::get(Context, Wrapped1Ops);
> > > +  ASSERT_EQ(Empty, Wrapped1->getOperand(0));
> > > +  ASSERT_TRUE(Wrapped1->isResolved());
> > > +  EXPECT_FALSE(Wrapped1->isDistinct());
> > > +
> > > +  // !{!{!{}}}
> > > +  Metadata *Wrapped2Ops[] = {Wrapped1};
> > > +  MDNode *Wrapped2 = MDNode::get(Context, Wrapped2Ops);
> > > +  ASSERT_EQ(Wrapped1, Wrapped2->getOperand(0));
> > > +  ASSERT_TRUE(Wrapped2->isResolved());
> > > +  EXPECT_FALSE(Wrapped2->isDistinct());
> > > +
> > > +  // !{!{!{}}} => !{!{}}
> > > +  Wrapped2->replaceOperandWith(0, Empty);
> > > +  ASSERT_EQ(Empty, Wrapped2->getOperand(0));
> > > +  EXPECT_TRUE(Wrapped2->isDistinct());
> > > +  EXPECT_FALSE(Wrapped1->isDistinct());
> > > +}
> > > +
> > >  typedef MetadataTest MetadataAsValueTest;
> > >
> > >  TEST_F(MetadataAsValueTest, MDNode) {
> > >
> > >
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150112/dd164519/attachment.html>