[llvm] r225738 - IR: Fix an inverted assertion when replacing resolved operands

[David Blaikie]

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

> Author: dexonsmith
> Date: Mon Jan 12 18:10:38 2015
> New Revision: 225738
>
> URL: http://llvm.org/viewvc/llvm-project?rev=225738&view=rev
> Log:
> IR: Fix an inverted assertion when replacing resolved operands
>
> Add a unit test, since this bug was only exposed by clang tests.  Thanks
> to Rafael for tracking this down!
>
> Modified:
>     llvm/trunk/lib/IR/Metadata.cpp
>     llvm/trunk/unittests/IR/MetadataTest.cpp
>
> Modified: llvm/trunk/lib/IR/Metadata.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=225738&r1=225737&r2=225738&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/IR/Metadata.cpp (original)
> +++ llvm/trunk/lib/IR/Metadata.cpp Mon Jan 12 18:10:38 2015
> @@ -444,7 +444,7 @@ void UniquableMDNode::resolveAfterOperan
>
>    // Check if an operand was resolved.
>    if (!isOperandUnresolved(Old))
> -    assert(isOperandUnresolved(New) && "Operand just became unresolved");
> +    assert(!isOperandUnresolved(New) && "Operand just became unresolved");
>

Could the if condition be rolled into the assertion here, or does it have
side effects? (tricksy name if it does)


>    else if (!isOperandUnresolved(New))
>      decrementUnresolvedOperandCount();
>  }
>
> Modified: llvm/trunk/unittests/IR/MetadataTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/MetadataTest.cpp?rev=225738&r1=225737&r2=225738&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/IR/MetadataTest.cpp (original)
> +++ llvm/trunk/unittests/IR/MetadataTest.cpp Mon Jan 12 18:10:38 2015
> @@ -7,6 +7,7 @@
>  //
>
>  //===----------------------------------------------------------------------===//
>
> +#include "llvm/ADT/STLExtras.h"
>  #include "llvm/IR/Metadata.h"
>  #include "llvm/IR/Constants.h"
>  #include "llvm/IR/Instructions.h"
> @@ -361,6 +362,28 @@ TEST_F(MDNodeTest, handleChangedOperandR
>    EXPECT_EQ(N4, N6->getOperand(0));
>  }
>
> +TEST_F(MDNodeTest, replaceResolvedOperand) {
> +  // Check code for replacing one resolved operand with another.  If
> doing this
> +  // directly (via replaceOperandWith()) becomes illegal, change the
> operand to
> +  // a global value that gets RAUW'ed.
> +  //
> +  // Use a temporary node to keep N from being resolved.
> +  std::unique_ptr<MDNodeFwdDecl> Temp(MDNodeFwdDecl::get(Context, None));
> +  Metadata *Ops[] = {nullptr, Temp.get()};
> +
> +  MDNode *Empty = MDTuple::get(Context, {});
> +  MDNode *N = MDTuple::get(Context, Ops);
> +  EXPECT_EQ(nullptr, N->getOperand(0));
> +  ASSERT_FALSE(N->isResolved());
> +
> +  // Check code for replacing resolved nodes.
> +  N->replaceOperandWith(0, Empty);
> +  EXPECT_EQ(Empty, N->getOperand(0));
> +
> +  // Remove the reference to Temp; required for teardown.
> +  N->replaceOperandWith(1, nullptr);
> +}
> +
>  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/52179a9d/attachment.html>