[PATCH] D74947: [unittests] demonstrate User::replaceUsesOfWith() breaking CallBrInst

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers created this revision.
nickdesaulniers added a reviewer: void.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
nickdesaulniers added a comment.
nickdesaulniers updated this revision to Diff 245786.
nickdesaulniers updated this revision to Diff 245787.
nickdesaulniers planned changes to this revision.

whoops, didn't mean to commit all that, let me drop some of this.


nickdesaulniers added a comment.

- drop mistakenly committed hacked up files


nickdesaulniers added a comment.

- git-clang-format HEAD~2


nickdesaulniers added a comment.

Can't land this test as is, because it would be red, due to `User::replaceUsesOfWith()` not updating the asm's argparam.


Also, refactor CallBrInstruction IRTest. Run via:

$ ninja IRTests
$ ./unittests/IR/IRTests --gtest_filter=InstructionsTest.CallBrInstruction

Link: https://github.com/ClangBuiltLinux/linux/issues/896


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74947

Files:
  llvm/unittests/IR/InstructionsTest.cpp


Index: llvm/unittests/IR/InstructionsTest.cpp
===================================================================
--- llvm/unittests/IR/InstructionsTest.cpp
+++ llvm/unittests/IR/InstructionsTest.cpp
@@ -1158,26 +1158,35 @@
   FNeg->deleteValue();
 }
 
+void VerifyCallBr(const CallBrInst &CBI, const BasicBlock &Expected,
+                  const std::string &F) {
+  BlockAddress *IndirectBA = BlockAddress::get(CBI.getIndirectDest(0));
+  BlockAddress *ArgBA = cast<BlockAddress>(CBI.getArgOperand(0));
+  EXPECT_EQ(IndirectBA, ArgBA)
+      << "After " << F << ", callbr had an indirect destination of '"
+      << CBI.getIndirectDest(0)->getName() << "', but a argument of '"
+      << ArgBA->getBasicBlock()->getName() << "'. These should always match:\n"
+      << CBI;
+  EXPECT_EQ(IndirectBA->getBasicBlock(), &Expected);
+  EXPECT_EQ(ArgBA->getBasicBlock(), &Expected);
+}
+
 TEST(InstructionsTest, CallBrInstruction) {
   LLVMContext Context;
   std::unique_ptr<Module> M = parseIR(Context, R"(
 define void @foo() {
 entry:
-  callbr void asm sideeffect "// XXX: ${0:l}", "X"(i8* blockaddress(@foo, %branch_test.exit))
+  callbr void asm sideeffect "", "X"(i8* blockaddress(@foo, %branch_test.exit))
           to label %land.rhs.i [label %branch_test.exit]
 
 land.rhs.i:
   br label %branch_test.exit
 
 branch_test.exit:
-  %0 = phi i1 [ true, %entry ], [ false, %land.rhs.i ]
-  br i1 %0, label %if.end, label %if.then
+  br label %if.then
 
 if.then:
   ret void
-
-if.end:
-  ret void
 }
 )");
   Function *Foo = M->getFunction("foo");
@@ -1197,16 +1206,11 @@
   // Further, test that changing the indirect destination updates the arg
   // operand to use the block address of the new indirect destination basic
   // block. This is a critical invariant of CallBrInst.
-  BlockAddress *IndirectBA = BlockAddress::get(CBI.getIndirectDest(0));
-  BlockAddress *ArgBA = cast<BlockAddress>(CBI.getArgOperand(0));
-  EXPECT_EQ(IndirectBA, ArgBA)
-      << "After setting the indirect destination, callbr had an indirect "
-         "destination of '"
-      << CBI.getIndirectDest(0)->getName() << "', but a argument of '"
-      << ArgBA->getBasicBlock()->getName() << "'. These should always match:\n"
-      << CBI;
-  EXPECT_EQ(IndirectBA->getBasicBlock(), &IfThen);
-  EXPECT_EQ(ArgBA->getBasicBlock(), &IfThen);
+  VerifyCallBr(CBI, IfThen, "CallBrInst::setIndirectDest()");
+
+  // Now try changing it back with User::replaceUsesOfWith().
+  CBI.replaceUsesOfWith(CBI.getIndirectDest(0), &BranchTestExit);
+  VerifyCallBr(CBI, BranchTestExit, "User::replaceUsesOfWith()");
 }
 
 TEST(InstructionsTest, UnaryOperator) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74947.245787.patch
Type: text/x-patch
Size: 2639 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200221/e4744ebc/attachment.bin>